|
Description
Tomás Cohen Arazi (tcohen)
2018-10-27 12:09:57 UTC
Created attachment 81360 [details] [review] Bug 21702: Make mancredit.pl use the patron id for the staff user I think unsafe SQL modes and the fact that manager_id has no FK allowed this to go unnoticed. But now we catch it! To test: - On a patron, try adding a new manual credit of any type => FAIL: It fails telling the userid of the logged user is not a valid integer! - Apply this patch - Try adding a manual credit of any type => SUCCESS: Manual credit added! - Sign off! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 81361 [details] [review] Bug 21702: (follow-up) Simplify checks and use standard fallback behaviour This patch acknowledges the fact that some checks here are done in a way that is not standard these days. This checks are originally done multiple times in different ways. output_and_exit_if_error should be used. This implies a minor behaviour change: if the passed (in the URL) borrowernumber doesn't exist, it sends the user to a 'Patron doesn't exist page' instead of the circulation page for the borrowernumber. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 81464 [details] [review] Bug 21702: Make mancredit.pl use the patron id for the staff user I think unsafe SQL modes and the fact that manager_id has no FK allowed this to go unnoticed. But now we catch it! To test: - On a patron, try adding a new manual credit of any type => FAIL: It fails telling the userid of the logged user is not a valid integer! - Apply this patch - Try adding a manual credit of any type => SUCCESS: Manual credit added! - Sign off! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 81465 [details] [review] Bug 21702: (follow-up) Simplify checks and use standard fallback behaviour This patch acknowledges the fact that some checks here are done in a way that is not standard these days. This checks are originally done multiple times in different ways. output_and_exit_if_error should be used. This implies a minor behaviour change: if the passed (in the URL) borrowernumber doesn't exist, it sends the user to a 'Patron doesn't exist page' instead of the circulation page for the borrowernumber. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Created attachment 81593 [details] [review] Bug 21702: Make mancredit.pl use the patron id for the staff user I think unsafe SQL modes and the fact that manager_id has no FK allowed this to go unnoticed. But now we catch it! To test: - On a patron, try adding a new manual credit of any type => FAIL: It fails telling the userid of the logged user is not a valid integer! - Apply this patch - Try adding a manual credit of any type => SUCCESS: Manual credit added! - Sign off! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 81594 [details] [review] Bug 21702: (follow-up) Simplify checks and use standard fallback behaviour This patch acknowledges the fact that some checks here are done in a way that is not standard these days. This checks are originally done multiple times in different ways. output_and_exit_if_error should be used. This implies a minor behaviour change: if the passed (in the URL) borrowernumber doesn't exist, it sends the user to a 'Patron doesn't exist page' instead of the circulation page for the borrowernumber. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Awesome work all! Pushed to master for 18.11 I'm not entirely confident in backporting this.. if it's required for 18.05.x series could you submit an alternative patch for it? Reworked version pushed to 18.05.x series for 18.05.06 Depends on bug 20980 not in 17.11.x |