https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #50 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Initial QA feedback: 1) http-client.js line 44 you're early returning if return_response is set but we already default every DELETE and COUNT request in all Vue modules to return this so those endpoints will now skip the existing error handling. Either needs its own checkin specific flag or the error handling needs re-thinking 2) XSS in Main.vue - the checkin message is customisable in the staff interface and is then put directly into v-html with no parsing. In the old TT version html_line_break was used to filter the message but we have no fallback here 3) Fetching the page can checkin an item - checkout an item and then paste the following into the browser with the barcode of that item: localhost:8081/cgi-bin/koha/circ/checkin.pl?barcode=<barcode>. A checkin will be handled silently and will appear in the database table but not in the UI. This used to be a CSRF protected POST but now fires on a page GET 4) No barcode filtering - Legacy code called barcodedecode - itemBarcodeInputFilter and the item_barcode_transform plugin hook also no longer apply to check-in through the Checkins.pm endpoints 5) Holds queue not updated - BatchUpdateBiblioHoldsQueue isn't called anywhere in the process flow and skip_holds_queue is passed through the AddReturn flow so the holds queue never reflects the check-in 6) audio-alert classes are added but playSound is never called after the initial page load 7) printAndResolveRecall, ConfirmationModal.vue line 966 - pulls the reserve_id from recallInfo but recallInfo doesn't carry a reserve_id property payload => { recall_id => $recall->id, needs_transfer => $messages->{RecallNeedsTransfer} ? 1 : 0, }, 8) xt/tt_script_tidy.t fails 9) No cypress tests - some of the existing Selenium tests are no longer robust, for example basic_workflow.t matches a static page title which exists before the Vue page has even mounted so is no longer actually proving anything. The Selenium tests either need updating or replacing with Cypress tests. This is a huge re-write for which good test coverage is critical 10) ModReserveAffect no longer receives a desk_id - any waiting hold will lose their desk 11) If MarkIssueReturned anonymises a patron record, when _attach_messages_to_checkin access $checkin->checkout->patron it is accessing the anonymous patron and not the actual patron involved. Any fines or waiting holds are incorrectly notified 12) Checkin.pm line 265 you throw an error based on the unless condition but ModReserveAffect can either return 0 or silently return if the hold isn't found (e.g. was deleted). This seems like if could incorrectly throw the exception in some cases 13) The /checkins/*_confirmation and /checkins/*_cancellation endpoints seem like they should probably require the same JWT confirmation flow as some of the work in bug 24401? These could be used to alter holds or transfers multiple days after they are created 14) Koha/Manual.pm needs updating to the new page URLs 15) ResolveClaimModal.vue uses the fetch api instead of the APIClient pattern. It also emits 'resolved' regardless of the response status which is then marked as a success further up the chain, even if a 4/500 error is thrown by the API 16) CheckinOptions.vue specifies MM/DD/YYYY which conflicts with Koha's date formatting options. The date is also no validated, should this use a flatpickr window? 17) checkin.js - fetch used for libraries which already have an APIClient - APIClient is already imported in the file. There are also multiple error strings throughout the file which aren't wrapped for translations - please check and update them 18) Missing translation wrapper in ConfirmationModal.vue line 799 19) TODO in ConfirmationModal.vue line seems like a regression/missing feature - is there a follow-up for this? 20) ConfirmationModal.vue line 799 - should this use patron_to_html to format the name? Same in CheckedInItems.vue line 442 21) recall_confirmation API method has no unit test 22) Policy.pm - the global keys are added to the hash before the subclass which means they can be overwritten. Should this not be reversed? 23) JWT is used for Module::Policy classes but the signature is never read in the UI. Either we need to check the signature or move away from JWT as it isn't adding anything -- You are receiving this mail because: You are watching all bug changes.