Bug 19791 - Patron Modification log redirects to circulation page
Summary: Patron Modification log redirects to circulation page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 19080
Blocks: 25249
  Show dependency treegraph
 
Reported: 2017-12-11 10:54 UTC by Abdou Yousuf
Modified: 2020-11-30 21:46 UTC (History)
8 users (show)

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


Attachments
Bug 19791: Keep user in circ when viewing patron's logs (5.54 KB, patch)
2020-02-14 10:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19791: Keep user in circ when viewing patron's logs (5.54 KB, patch)
2020-02-14 16:05 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19791: Keep user in circ when viewing patron's logs (5.65 KB, patch)
2020-02-18 19:23 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Abdou Yousuf 2017-12-11 10:54:05 UTC
Dear all,

- Showing patron Modification log from koha 17.11 doesn't show any system log
for the patron, Instead it redirects to 
(http://domain_name/cgi-bin/koha/circ/circulation.pl?borrowernumber=)

- Just try filling the (Librarian:) field with the borrower number instead of the (Object:) field, And leave the (Object:)field empty, You will see what I mean.

- Also you can test it here
(http://koha.adminkuhn.ch:8080/cgi-bin/koha/tools/viewlog.pl?do_it=1&modules=MEMBERS&modules=circulation&object=1&src=circ), It's a demo site for koha 17.11 provided by koha community here (https://koha-community.org/demo/)  use (demo/demo) for logging in, Then test it for this user (Herr Demokrit Morgenstern (1)), Also the problem was not in koha 17.05, You can test it using
Bywater demo version (koha17.05), You will see that it working fine in koha
(17.05).
Comment 1 Jonathan Druart 2017-12-11 15:23:43 UTC
With bug 19080 we tried to redirect to the circulation home page when the borrowernumber did not exist (anymore).

When you click on the "modification log" tab, the "object" parameter is automatically filled with the borrowernumber of the patron you are looking at.

If you decide to empty this field or use a non-existent borrowernumber, you will get redirected to the circulation home page.

I would suggest to:
- Disable the "object" input and force the value to borrowernumber when we come from the circulation module and redirect to circ home page if non-existent
- Make it editable when come from the tools module and display a "no logs" message if non-existent borrowernumber is used.
Comment 2 Abdou Yousuf 2017-12-11 17:14:46 UTC
(In reply to Jonathan Druart from comment #1)
> With bug 19080 we tried to redirect to the circulation home page when the
> borrowernumber did not exist (anymore).
> 
> When you click on the "modification log" tab, the "object" parameter is
> automatically filled with the borrowernumber of the patron you are looking
> at.
> 
> If you decide to empty this field or use a non-existent borrowernumber, you
> will get redirected to the circulation home page.
> 
> I would suggest to:
> - Disable the "object" input and force the value to borrowernumber when we
> come from the circulation module and redirect to circ home page if
> non-existent
> - Make it editable when come from the tools module and display a "no logs"
> message if non-existent borrowernumber is used.

I don't think that it's beneficial to automatically fill (Object field) with the borrower number, On the contrary I think that the (Librarian:) field should be automatically filled by borrower number rather than (Object field), And it will be more beneficial, In fact I don't know why borrower number should be in (Object field) and not (Librarian field) when selecting "modification log" tab, What are the benefits?

Also, You said that you tried to redirect to the circulation home page when the
borrower number did not exist (anymore), How it can be? I think it's not logical even though it can be done programmatically.

Regards.
Comment 3 Josef Moravec 2017-12-12 07:44:40 UTC
(In reply to Abdou Yousuf from comment #2)
> (In reply to Jonathan Druart from comment #1)
> > With bug 19080 we tried to redirect to the circulation home page when the
> > borrowernumber did not exist (anymore).
> > 
> > When you click on the "modification log" tab, the "object" parameter is
> > automatically filled with the borrowernumber of the patron you are looking
> > at.
> > 
> > If you decide to empty this field or use a non-existent borrowernumber, you
> > will get redirected to the circulation home page.
> > 
> > I would suggest to:
> > - Disable the "object" input and force the value to borrowernumber when we
> > come from the circulation module and redirect to circ home page if
> > non-existent
> > - Make it editable when come from the tools module and display a "no logs"
> > message if non-existent borrowernumber is used.
> 
> I don't think that it's beneficial to automatically fill (Object field) with
> the borrower number, On the contrary I think that the (Librarian:) field
> should be automatically filled by borrower number rather than (Object
> field), And it will be more beneficial, In fact I don't know why borrower
> number should be in (Object field) and not (Librarian field) when selecting
> "modification log" tab, What are the benefits?

The tabs on patron pages are all relevant to current patron, that is the reason why borrowernumber is in object - you want here to get information about this patron. 

> Also, You said that you tried to redirect to the circulation home page when
> the
> borrower number did not exist (anymore), How it can be? I think it's not
> logical even though it can be done programmatically.

It could be possible - for example one librarian deletes a patron and the second librarian wants to show some information about patron. 

> 
> Regards.
Comment 4 Katrin Fischer 2018-03-31 10:45:49 UTC
> I would suggest to:
> - Disable the "object" input and force the value to borrowernumber when we
> come from the circulation module and redirect to circ home page if
> non-existent
> - Make it editable when come from the tools module and display a "no logs"
> message if non-existent borrowernumber is used.

That would work for me. Also agree with Josef that using the borrowernumber of the patron in object is the expected behaviour.
Comment 5 Jonathan Druart 2020-02-14 10:48:13 UTC
Created attachment 98922 [details] [review]
Bug 19791: Keep user in circ when viewing patron's logs

When a librarian is viewing the log for a given patron (tab "modification log"
from the circulation module), the "object" and "modules" parameter can
be modified. It does not make sense, we should stay in the circulation
module and avoid those fields to be modified.

Test plan:
- Modify several patrons (in order to have several modification logs)
- Go to the modification log tab of one of them
- Notice that the "Object" input is not a "Borrowernumber" labelled
input and that you cannot modify it.
Also notice that the "modules" is hidden.
- Go to the log viewer tool and notice that the view is still the same
as before this patch.
Comment 6 Owen Leonard 2020-02-14 16:05:51 UTC
Created attachment 98967 [details] [review]
Bug 19791: Keep user in circ when viewing patron's logs

When a librarian is viewing the log for a given patron (tab "modification log"
from the circulation module), the "object" and "modules" parameter can
be modified. It does not make sense, we should stay in the circulation
module and avoid those fields to be modified.

Test plan:
- Modify several patrons (in order to have several modification logs)
- Go to the modification log tab of one of them
- Notice that the "Object" input is not a "Borrowernumber" labelled
input and that you cannot modify it.
Also notice that the "modules" is hidden.
- Go to the log viewer tool and notice that the view is still the same
as before this patch.
Comment 7 Martin Renvoize 2020-02-18 19:23:27 UTC
Created attachment 99220 [details] [review]
Bug 19791: Keep user in circ when viewing patron's logs

When a librarian is viewing the log for a given patron (tab "modification log"
from the circulation module), the "object" and "modules" parameter can
be modified. It does not make sense, we should stay in the circulation
module and avoid those fields to be modified.

Test plan:
- Modify several patrons (in order to have several modification logs)
- Go to the modification log tab of one of them
- Notice that the "Object" input is not a "Borrowernumber" labelled
input and that you cannot modify it.
Also notice that the "modules" is hidden.
- Go to the log viewer tool and notice that the view is still the same
as before this patch.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2020-02-18 19:24:51 UTC
Works as expected, no regressions found and QA scripts pass.. Passing QA
Comment 9 Martin Renvoize 2020-02-19 11:34:07 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 10 Joy Nelson 2020-03-13 00:36:19 UTC
Pushed to 19.11.x branch for 19.11.04
Comment 11 Lucas Gass 2020-03-18 21:43:24 UTC
backported to 19.05.x for 19.05.09