Bug 17397

Summary: Show name of librarian who created circulation message
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: CirculationAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, koha, kyle.m.hall, kyle, mtj, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17398, 17558    
Attachments: Bug 17397: Show name of librarian who created circulation message
Bug 17397: Show name of librarian who created circulation message
Bug 17397: Show name of librarian who created circulation message
Bug 17397: Show name of librarian who created circulation message
Bug 17397: Show name of librarian who created circulation message
Bug 17397: Show name of librarian who created circulation message
Bug 17397: Simplify code passing the Koha::Object to the template
Bug 17397: Do not display parenthesis if the manager has been deleted

Description Josef Moravec 2016-10-04 11:54:57 UTC

    
Comment 1 Josef Moravec 2016-10-04 12:04:02 UTC Comment hidden (obsolete)
Comment 2 Marc VĂ©ron 2016-10-04 15:15:02 UTC
Sorry, QA-Tools complain: 

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
(...)
   FAIL	  tt_valid
		lines 855

See: 
https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML
Comment 3 Josef Moravec 2016-10-05 05:23:46 UTC Comment hidden (obsolete)
Comment 4 Josef Moravec 2016-10-05 05:33:21 UTC Comment hidden (obsolete)
Comment 5 Josef Moravec 2016-10-05 05:35:07 UTC
Created attachment 56046 [details] [review]
Bug 17397: Show name of librarian who created circulation message

Test plan:
1) Apply the patch
2) Update DB structure
3) Run update_dbix_class_files.pl
4) Select patron for checking out
5) Try to add some circulation and opac messages
6) Note that now there is creator (you ;) ) shown by every message added (with link to creator profile)
7) Try to delete messages to confirm that everything works as expected
Comment 6 Josef Moravec 2016-10-05 05:35:59 UTC
Sorry, now Koha QA tools passed cleanly
Comment 7 Koha Team University Lyon 3 2016-10-14 10:16:01 UTC
I have tried to add different messages from different libraries staff user.
Names appears correctly in pro interface and libraries in OPAC interface.
Message are correctly deleted.
It respects the syspref AllowAllMessageDeletion.
Comment 8 Koha Team University Lyon 3 2016-10-14 10:21:24 UTC
Created attachment 56503 [details] [review]
Bug 17397: Show name of librarian who created circulation message

Test plan:
1) Apply the patch
2) Update DB structure
3) Run update_dbix_class_files.pl
4) Select patron for checking out
5) Try to add some circulation and opac messages
6) Note that now there is creator (you ;) ) shown by every message added (with link to creator profile)
7) Try to delete messages to confirm that everything works as expected

Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>
Comment 9 Jonathan Druart 2016-10-25 08:08:26 UTC
Comment on attachment 56503 [details] [review]
Bug 17397: Show name of librarian who created circulation message

Review of attachment 56503 [details] [review]:
-----------------------------------------------------------------

::: Koha/Patron/Message.pm
@@ +49,5 @@
>                and $self->message_type
>                and $self->branchcode;
>  
> +    my $userenv = C4::Context->userenv;
> +    $self->manager_id($userenv ? $userenv->{number} : 0);

Please provide tests.

::: circ/circulation.pl
@@ +587,5 @@
> +    $this_item->{message}        = $content->message;
> +    $this_item->{manager_id}     = $content->manager_id;
> +    $this_item->{name}           = $content->_result->get_column('manager_firstname') . ' ' . $content->_result->get_column('manager_surname');
> +
> +    push @messages, $this_item;

You should not need this loop. You can access the message's methods from the template.
Comment 10 Josef Moravec 2016-10-26 19:59:14 UTC
> @@ +587,5 @@
> > +    $this_item->{message}        = $content->message;
> > +    $this_item->{manager_id}     = $content->manager_id;
> > +    $this_item->{name}           = $content->_result->get_column('manager_firstname') . ' ' . $content->_result->get_column('manager_surname');
> > +
> > +    push @messages, $this_item;
> 
> You should not need this loop. You can access the message's methods from the
> template.

I have written some tests and now I tried to eliminate that loop. I am able to access columns from table messages, but I can't access the two columns which are get by relation to table borrowers - manager_firstname and manager_surname... I am probably doing something wrong...
Comment 11 Jonathan Druart 2016-10-27 07:44:04 UTC
Created attachment 56911 [details] [review]
Bug 17397: Show name of librarian who created circulation message

Test plan:
1) Apply the patch
2) Update DB structure
3) Run update_dbix_class_files.pl
4) Select patron for checking out
5) Try to add some circulation and opac messages
6) Note that now there is creator (you ;) ) shown by every message added (with link to creator profile)
7) Try to delete messages to confirm that everything works as expected

Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 12 Jonathan Druart 2016-10-27 07:44:07 UTC
Created attachment 56912 [details] [review]
Bug 17397: Simplify code passing the Koha::Object to the template

Instead of creating a new array

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 13 Jonathan Druart 2016-10-27 07:44:14 UTC
Created attachment 56913 [details] [review]
Bug 17397: Do not display parenthesis if the manager has been deleted

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 14 Kyle M Hall 2016-10-28 15:45:15 UTC
Pushed to master for 16.11, thanks Josef, Jonathan!
Comment 15 Jonathan Druart 2016-10-31 10:30:11 UTC
(In reply to Kyle M Hall from comment #14)
> Pushed to master for 16.11, thanks Josef, Jonathan!

The DBix::Class schema is not up-to-date
Comment 16 Kyle M Hall 2016-10-31 10:43:56 UTC
(In reply to Jonathan Druart from comment #15)
> (In reply to Kyle M Hall from comment #14)
> > Pushed to master for 16.11, thanks Josef, Jonathan!
> 
> The DBix::Class schema is not up-to-date

Fixed!
Comment 17 Jonathan Druart 2016-11-04 15:20:30 UTC
This patch set seems to break t/db_dependent/Koha/Patron/Messages.t, please fix.
Comment 18 Josef Moravec 2016-11-04 15:58:46 UTC
I'll work on it in just few hours, I am pretty sure I have some test code on my another machine...
Comment 19 Josef Moravec 2016-11-04 18:55:39 UTC
I added fix to new bug 17558
Comment 20 Mason James 2016-11-30 11:07:02 UTC
(In reply to Kyle M Hall from comment #14)
> Pushed to master for 16.11, thanks Josef, Jonathan!

contains DB changes, passing for 16.05