Bug 17397 - Show name of librarian who created circulation message
Summary: Show name of librarian who created circulation message
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Josef Moravec
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 17398 17558
  Show dependency treegraph
 
Reported: 2016-10-04 11:54 UTC by Josef Moravec
Modified: 2017-06-14 22:16 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17397: Show name of librarian who created circulation message (6.76 KB, patch)
2016-10-04 12:04 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17397: Show name of librarian who created circulation message (6.87 KB, patch)
2016-10-05 05:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17397: Show name of librarian who created circulation message (6.87 KB, patch)
2016-10-05 05:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17397: Show name of librarian who created circulation message (6.87 KB, patch)
2016-10-05 05:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17397: Show name of librarian who created circulation message (6.92 KB, patch)
2016-10-14 10:21 UTC, Koha Team University Lyon 3
Details | Diff | Splinter Review
Bug 17397: Show name of librarian who created circulation message (6.98 KB, patch)
2016-10-27 07:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17397: Simplify code passing the Koha::Object to the template (3.21 KB, patch)
2016-10-27 07:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17397: Do not display parenthesis if the manager has been deleted (1.66 KB, patch)
2016-10-27 07:44 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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