The changes on bug 40319 should be covered by tests.
Created attachment 183843 [details] [review] Bug 40320: (bug 40319 follow-up) Add Cypress tests Test plan: Make sure the display of city, state and zipcode is the same as before the patch Confirm that the new Cypress tests pass
Created attachment 183940 [details] [review] Bug 40320: (bug 40319 follow-up) Add Cypress tests Test plan: Make sure the display of city, state and zipcode is the same as before the patch Confirm that the new Cypress tests pass
Created attachment 183944 [details] [review] Bug 40320: (bug 40319 follow-up) Add Cypress tests Test plan: Make sure the display of city, state and zipcode is the same as before the patch Confirm that the new Cypress tests pass Signed-off-by: Owen Leonard <oleonard@myacpl.org>
[FAIL] koha-tmpl/intranet-tmpl/prog/en/includes/member-display-address-style.inc FAIL filters missing_filter at line 34 ( [%~ IF ( patron.city ) ~%][% ", " %][% END ~%])
Created attachment 184369 [details] [review] Bug 40320: (bug 40319 follow-up) Add Cypress tests Test plan: Make sure the display of city, state and zipcode is the same as before the patch Confirm that the new Cypress tests pass Signed-off-by: Owen Leonard <oleonard@myacpl.org> Amended-by: Jonathan Druart Added $raw filter to [% ", " %]
Created attachment 186958 [details] [review] Bug 40320: (bug 40319 follow-up) Add Cypress tests Test plan: Make sure the display of city, state and zipcode is the same as before the patch Confirm that the new Cypress tests pass Signed-off-by: Owen Leonard <oleonard@myacpl.org> Amended-by: Jonathan Druart Added $raw filter to [% ", " %] Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
No blocker just a comment: + [%~ IF ( patron.city ) ~%][% ", " | $raw %][% END ~%] This looks overcomplicated to me somehow. Note that the second tilde in the IF should do nothing. And what is the bonus of pushing a comma thru raw anyway?
(In reply to Marcel de Rooy from comment #7) > No blocker just a comment: > + [%~ IF ( patron.city ) ~%][% ", " | $raw %][% END ~%] > This looks overcomplicated to me somehow. > Note that the second tilde in the IF should do nothing. And what is the > bonus of pushing a comma thru raw anyway? IIRC We want to keep the space after the comma. If you have [% IF %], [% END %] then prettier will remove the space. Use TT [% ", " %] makes sure the space will stay. The filter is mandatory or find-missing-filters.t will fail (and better raw than html, we know there is nothing to escape). You can play with the code and the tests if you think there is a better solution.
(In reply to Jonathan Druart from comment #8) > (In reply to Marcel de Rooy from comment #7) > > No blocker just a comment: > > + [%~ IF ( patron.city ) ~%][% ", " | $raw %][% END ~%] > > This looks overcomplicated to me somehow. > > Note that the second tilde in the IF should do nothing. And what is the > > bonus of pushing a comma thru raw anyway? > > IIRC We want to keep the space after the comma. > > If you have [% IF %], [% END %] then prettier will remove the space. > Use TT [% ", " %] makes sure the space will stay. > The filter is mandatory or find-missing-filters.t will fail (and better raw > than html, we know there is nothing to escape). > > You can play with the code and the tests if you think there is a better > solution. prettier was not yet on my radar. No play, just push
Created attachment 187013 [details] [review] Bug 40320: Use insertSamplePatron On commit 959b7e8f05059522151fb24992ed7250ac436dc1 Bug 40345: Add insertSamplePatron We remove the ability to insert a patron using insertObject, we need to use insertSamplePatron instead.
Nice work everyone! Pushed to main for 25.11