Bug 21242 - Modification log redirects you to circulation with no borrower if 'Object' field is not populated with borrowernumber
Summary: Modification log redirects you to circulation with no borrower if 'Object' fi...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 18789 21634
Blocks:
  Show dependency treegraph
 
Reported: 2018-08-17 11:58 UTC by Nick Clemens
Modified: 2018-11-28 12:50 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21242: If not searching for patron, move to log viewer (1.35 KB, patch)
2018-08-17 12:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21242: If not searching for patron, move to log viewer (1.40 KB, patch)
2018-08-17 16:33 UTC, Owen Leonard
Details | Diff | Splinter Review
[ALTERNATIVE-PATCH] Bug 21242: Make object's input read-only when viewing patron's logs (2.31 KB, patch)
2018-08-17 17:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21242: If not searching for patron, move to log viewer (1.47 KB, patch)
2018-10-28 17:02 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-08-17 11:58:52 UTC
The code seems to make the assumption that if we come from circ, we only want to view the borrowers info, however, it provides the full form, indicating you can search for more info.


To recreate:
1 - Pull up a patron
2 - Click 'Modification log'
3 - You see info about patron
4 - Change the form, moving Object to Info
5 - You are sent to a blank circ page
Comment 1 Nick Clemens 2018-08-17 12:28:22 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2018-08-17 16:33:50 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2018-08-17 17:43:06 UTC
And what about making "object" read-only if viewing logs from a given patron?

https://snag.gy/rqolXT.jpg
Comment 4 Jonathan Druart 2018-08-17 17:44:01 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2018-10-23 13:06:44 UTC
Nick, Owen, what about this alternative patch?
Comment 6 Nick Clemens 2018-10-23 13:48:55 UTC
(In reply to Jonathan Druart from comment #5)
> Nick, Owen, what about this alternative patch?

Hmm...this still allows for odd things - the borrowernumber is locked, but I can change the moduel to "Catalog", then I get the info for the biblio with the same number, and I have circ options with no patron.

If we want to go that route I would suggest to disable the whole form and replacing it with a link to the log viewer

I don't hate my patch though :-b
Comment 7 Jonathan Druart 2018-10-23 14:03:54 UTC
(In reply to Nick Clemens from comment #6)
> I don't hate my patch though :-b

Ok, let's ask for another QA point of view.
Comment 8 Katrin Fischer 2018-10-28 16:54:53 UTC
This is a hard one.

The basic problem seems to be that we use the log viewer in a specific context: related to one patron. But as it's a general search form there is no guarantee that people won't search for other things as well.

I don't like the alternative patch so much as it feels too limiting without reason. Why do we not allow it to change and force users to make more clicks if it's the same form they get when coming from tools?

I don't have a perfect solution, but I feel Nick's patch is an improvement over the current behavior.
Comment 9 Katrin Fischer 2018-10-28 17:02:19 UTC
Created attachment 81418 [details] [review]
Bug 21242: If not searching for patron, move to log viewer

To test:
1 - Bring up a patron
2 - Click 'Modification log'
3 - Note you get borrower info
4 - Move the object field into info field
5 - Submit
6 - You are now on the 'Log viewer' page with admin options, not
borrower circ options

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Nick Clemens 2018-10-29 01:19:45 UTC
Awesome work all!

Pushed to master for 18.11
Comment 11 Martin Renvoize 2018-11-08 15:52:14 UTC
Pushed to 18.05.x for 18.05.06
Comment 12 Fridolin SOMERS 2018-11-28 12:50:36 UTC
Patch depends on Bug 18789.
Since it is a minor impact, I choose not to push to 17.11.x