https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10190 --- Comment #953 from Jonathan Druart <jonathan.druart@gmail.com> --- Very quick, and late, QA review: 1. admin/circulation_triggers.tt 34 const letters = [% To.json(letters) | $raw %]; => XSS? 2. admin/circulation_triggers.tt 37 const logged_in_library_id = "[% Branches.GetLoggedInBranchcode | html %]"; We have a global logged_in_user JS variable already 3. Koha/REST/V1/CirculationRules.pm 120 return $c->render_invalid_parameter_value( 121 { 122 path => '/query/rules', 123 values => { 124 uri => '/api/v1/kinds', 125 field => 'rule_name' 126 } 127 } 128 ) unless $valid_kinds->{$rule_key}; Is this correct? /kinds is not defined in the api specs. Should it be /circulation_rules/kinds? 4. Certainly a personal pov but the code in Koha/REST/V1/CirculationRules.pm starting at l.244 (`my $logged_in_user = $c->stash('koha.user');`) to return the 403 is confusing. Too many return statements maybe? I am also not sure it's covered properly by tests (but I may have missed them). 5. Why do we still have occurrences of "edit_notice_status_triggers" in t/Koha/Auth/Permissions.t? t/Koha/Auth/Permissions.t:# Remove the deprecated edit_notice_status_triggers permission (bug 10190). t/Koha/Auth/Permissions.t:C4::Context->dbh->do(q|DELETE FROM permissions WHERE code = 'edit_notice_status_triggers'|); 6. i18n a. CirculationTriggersFormConfirmTriggerDelete.vue Untranslatable strings? + :formTitle=" + initialized + ? `Confirm deletion of Trigger ${triggerNumber} for ${handleContext(library_id, libraries, 'library_id')}` + : 'Confirm circulation trigger deletion' + " b. EditActions.vue + {{ $__("Add new trigger") + " " + triggerNumber }} + {{ $__("Edit trigger") }} {{ " " + triggerNumber }} => Should use `$__x()` to correctly handle this translation. Same pattern exist in other files. 7. EditActions.vue use an svg icon, any good reason or that? Doesn't it exist in font-awesome? 8. Duplicated CSS rules a. Just one example, but there are many like that: EditActions.vue CirculationTriggersFormConfirmTriggerDelete.vue CirculationTriggersFormConfirmReset.vue All 3 have the same CSS rules for `.chevron-buttons`, and only EditActions.vue have the .chevron-buttons element in the template. b. v-select related rules already exist in the shared css/vue.css (see "Bug 38201: Do not duplicated CSS rules") 9. In (at least) EditNotice.vue, there are !== conditions that can certainly be simplified: `v-if !== '' && !== null && !== undefined` => simply "v-if"? or do we care about 0? If we care about 0, then `!== null, !== undefined` and `!= null` are equivalent. 10. I don't understand why all the functions are in the stores file, is this a vue best practice? -- You are receiving this mail because: You are watching all bug changes.