[Bug 22043] New: SIP Checkin Response alert flag set to often set to Y incorrectly
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Bug ID: 22043 Summary: SIP Checkin Response alert flag set to often set to Y incorrectly Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com Target Milestone: --- If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 83468 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83468&action=edit Bug 22043: SIP Checkin Response alert flag set to often set to Y incorrectly If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field.If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field. Test Plan: 1) Set the checked_in_ok sip account flag to 1 for an account 2) Check in an already checked in item via SIP 3) Note the alert flag is set to Y 4) Apply this patch 5) Restart SIP 6) Check the item in again 7) Note the alert flag is set to N -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22016 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22016 [Bug 22016] Always send CT field for SIP checkin, even if empty -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Sally Healey <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83468|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 83578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83578&action=edit Bug 22043: SIP Checkin Response alert flag set to often set to Y incorrectly If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field.If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field. Test Plan: 1) Set the checked_in_ok sip account flag to 1 for an account 2) Check in an already checked in item via SIP 3) Note the alert flag is set to Y 4) Apply this patch 5) Restart SIP 6) Check the item in again 7) Note the alert flag is set to N Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jill Kleven <jill.kleven@pueblolibrary.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: - $self->alert(!$return); Although this seems to be okay, I am not fully sure about it. Do we overlook some codes? Or will this present a problem if we add a code but forgot to update SIP? @Colin: What is your opinion about this one ? Temporary parking into FQA for feedback. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Still waiting on feedback. Can we get this moving forward if we don't hear back by the end of the week? I can move the functionality behind a server parameter if necessary, but I as far as I can tell we are violating the standard with our current behavior. (In reply to Marcel de Rooy from comment #3)
QA Comment:
- $self->alert(!$return);
Although this seems to be okay, I am not fully sure about it. Do we overlook some codes? Or will this present a problem if we add a code but forgot to update SIP? @Colin: What is your opinion about this one ?
Temporary parking into FQA for feedback.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #5 from Colin Campbell <colin.campbell@ptfs-europe.com> --- (In reply to Kyle M Hall from comment #4)
Still waiting on feedback. Can we get this moving forward if we don't hear back by the end of the week? I can move the functionality behind a server parameter if necessary, but I as far as I can tell we are violating the standard with our current behavior.
(In reply to Marcel de Rooy from comment #3)
QA Comment:
- $self->alert(!$return);
Although this seems to be okay, I am not fully sure about it. Do we overlook some codes? Or will this present a problem if we add a code but forgot to update SIP? @Colin: What is your opinion about this one ?
Temporary parking into FQA for feedback.
No it doesnt violate the standard, the link between there being a message and the alert is a convention that the original code adopted. Similarly the assumption that CV must be present, CV was not part of the original spec, and will be ignored by some clients. Unfortunately its become more complex as since then we've added/suppressed various messages in the circ routines, and informational messages have been added. That said I suspect nowhere is reliant on the alert flag's current behaviour. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- In that case, I'll add a SIP config parameter to make this configurable. (In reply to Colin Campbell from comment #5)
No it doesnt violate the standard, the link between there being a message and the alert is a convention that the original code adopted. Similarly the assumption that CV must be present, CV was not part of the original spec, and will be ignored by some clients. Unfortunately its become more complex as since then we've added/suppressed various messages in the circ routines, and informational messages have been added. That said I suspect nowhere is reliant on the alert flag's current behaviour.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 85884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85884&action=edit Bug 22043: (QA Follow-up) Add paramter to control behavior -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Bug 22043 depends on bug 22016, which changed state. Bug 22016 Summary: Always send CT field for SIP checkin, even if empty https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22016 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85884|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 85897 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85897&action=edit Bug 22043: (QA Follow-up) Add paramter to control behavior -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83578|0 |1 is obsolete| | Attachment #85897|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 86668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86668&action=edit Bug 22043: SIP Checkin Response alert flag set to often set to Y incorrectly If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field.If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field. Test Plan: 1) Set the checked_in_ok sip account flag to 1 for an account 2) Check in an already checked in item via SIP 3) Note the alert flag is set to Y 4) Apply this patch 5) Restart SIP 6) Check the item in again 7) Note the alert flag is set to N Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jill Kleven <jill.kleven@pueblolibrary.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 86669 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86669&action=edit Bug 22043: (QA follow-up) Add parameter to control behavior Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- t/db_dependent/SIP/Message.t | 21 ++++++++++++++++++++- Great ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED CC| |martin.renvoize@ptfs-europe | |.com --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Bug with enhancement not in 18.11.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22076 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22076 [Bug 22076] SIP checkin for withdrawn item returns ok in checkin response -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22043 --- Comment #14 from Colin Campbell <colin.campbell@ptfs-europe.com> --- *** Bug 21218 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org