Bug 40320 - Missing Cypress tests for patron address display
Summary: Missing Cypress tests for patron address display
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 40181 40319
Blocks:
  Show dependency treegraph
 
Reported: 2025-07-07 15:01 UTC by Jonathan Druart
Modified: 2025-09-29 20:07 UTC (History)
2 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 40320: (bug 40319 follow-up) Add Cypress tests (3.26 KB, patch)
2025-07-07 15:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40320: (bug 40319 follow-up) Add Cypress tests (3.27 KB, patch)
2025-07-10 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40320: (bug 40319 follow-up) Add Cypress tests (3.32 KB, patch)
2025-07-10 13:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 40320: (bug 40319 follow-up) Add Cypress tests (3.38 KB, patch)
2025-07-18 12:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 40320: (bug 40319 follow-up) Add Cypress tests (3.47 KB, patch)
2025-09-26 07:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 40320: Use insertSamplePatron (2.50 KB, patch)
2025-09-26 19:43 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-07-07 15:01:05 UTC
The changes on bug 40319 should be covered by tests.
Comment 1 Jonathan Druart 2025-07-07 15:04:23 UTC
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
Comment 2 Jonathan Druart 2025-07-10 12:54:20 UTC
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
Comment 3 Owen Leonard 2025-07-10 13:58:51 UTC
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>
Comment 4 Martin Renvoize (ashimema) 2025-07-18 11:34:20 UTC
[FAIL] koha-tmpl/intranet-tmpl/prog/en/includes/member-display-address-style.inc
   FAIL   filters
                missing_filter at line 34 (                [%~ IF ( patron.city ) ~%][% ", " %][% END ~%])
Comment 5 Jonathan Druart 2025-07-18 12:18:26 UTC
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 [% ", " %]
Comment 6 Marcel de Rooy 2025-09-26 07:22:30 UTC
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>
Comment 7 Marcel de Rooy 2025-09-26 07:23:07 UTC
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?
Comment 8 Jonathan Druart 2025-09-26 07:54:58 UTC
(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.
Comment 9 Marcel de Rooy 2025-09-26 08:02:02 UTC
(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
Comment 10 Jonathan Druart 2025-09-26 19:43:32 UTC
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.
Comment 11 Lucas Gass (lukeg) 2025-09-29 20:07:49 UTC
Nice work everyone!

Pushed to main for 25.11