Bug 33528 - Use template wrapper for tabs: Patron details and circulation
Summary: Use template wrapper for tabs: Patron details and circulation
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL:
Keywords:
: 33527 (view as bug list)
Depends on: 32571 33945
Blocks: 34294 35987 36292 34402 34891
  Show dependency treegraph
 
Reported: 2023-04-13 14:56 UTC by Owen Leonard
Modified: 2024-03-18 09:19 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02
Circulation function:


Attachments
Bug 33528: Use template wrapper for tabs: Patron details page (74.17 KB, patch)
2023-05-16 16:05 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33528: Use template wrapper for tabs: Patron details page (74.20 KB, patch)
2023-05-31 15:47 UTC, Owen Leonard
Details | Diff | Splinter Review
Screenshot of display (39.24 KB, image/png)
2023-06-01 14:45 UTC, Lucas Gass (lukeg)
Details
Bug 33528: Use template wrapper for tabs: Patron details page (77.01 KB, patch)
2023-06-02 12:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33528: Use template wrapper for tabs: Patron details page (77.06 KB, patch)
2023-06-02 14:41 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33528: Use template wrapper for tabs: Patron details page (77.11 KB, patch)
2023-06-08 19:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33528: Use template wrapper for tabs: Patron details page (76.04 KB, patch)
2023-06-13 11:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33528: (follow-up) Correct selector for event handler (1.15 KB, patch)
2023-06-13 11:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33528: Fix basic_workflow.t (1.12 KB, patch)
2023-06-14 15:47 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2023-04-13 14:56:21 UTC
Continuing the work of Bug 32571, I would like to update the patron details page in the staff client to use the new WRAPPER for tabs
Comment 1 Owen Leonard 2023-05-16 16:05:51 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2023-05-16 16:08:47 UTC
Not expecting this to go into 23.05 but I thought I'd submit it in case someone can start testing it.
Comment 3 Owen Leonard 2023-05-16 16:09:38 UTC
*** Bug 33527 has been marked as a duplicate of this bug. ***
Comment 4 Owen Leonard 2023-05-31 15:47:34 UTC Comment hidden (obsolete)
Comment 5 Sam Lau 2023-05-31 18:27:43 UTC
I ran into some problems testing the club tab in both the check out page and the details page. The club shows up fine, however after clicking the enroll button, the "finish enrollment" button appears on the screen visually plain text rather than a button. And after clicking on the "finish enrollment" text, the club tab essentially disappears but all of the information from the tab is left on the screen. From here if you try to click on another tab, the formatting really gets messed up and the table turns green for some reason. Refreshing the page did put the club tab back in the correct spot and also fixed the formatting, but if you try to cancel your enrollment in the club, a pop up appears that says: "Something went wrong when loading the table." After closing the popup, the formatting of the club tab gets messed up again, with the same problems as described before.
Comment 6 Owen Leonard 2023-06-01 11:44:48 UTC
(In reply to Sam Lau from comment #5)
> I ran into some problems testing the club tab in both the check out page and
> the details page. 

I am not seeing this in my test system. Could you try again? Are there any JavaScript errors in the console?
Comment 7 Lucas Gass (lukeg) 2023-06-01 14:44:57 UTC
(In reply to Owen Leonard from comment #6)
> (In reply to Sam Lau from comment #5)
> > I ran into some problems testing the club tab in both the check out page and
> > the details page. 
> 
> I am not seeing this in my test system. Could you try again? Are there any
> JavaScript errors in the console?

I also recreate this. Steps I used:

1. Apply patch
2. Go to patron checkout or detail tab
3. Click on Clubs tab > Enroll
4. Above the table click 'Finish enrollment' or 'Cancel'
5. Clubs panel now appears in unusual position.
6. Click on another tab (Checkouts, Holds ) and the Clubs panel now has a green background.
Comment 8 Lucas Gass (lukeg) 2023-06-01 14:45:38 UTC
Created attachment 151935 [details]
Screenshot of display
Comment 9 Sam Lau 2023-06-01 14:54:31 UTC
Here is a video of the bug as well: https://www.youtube.com/watch?v=bcNWwtvDuVc
Comment 10 Owen Leonard 2023-06-02 12:56:19 UTC
Created attachment 151980 [details] [review]
Bug 33528: Use template wrapper for tabs: Patron details page

This patch updates the checkout and patron details templates so
that they use the new WRAPPER directive to build tabbed navigation.

The markup for the tab navigation and tab panels is moved into its own
include file since the templates were duplicating essentially the same
code.

To fully test you should have patrons with checkouts, fines, holds,
recalls, and article requests.

Apply the patch and go to the checkout page in the staff interface. With
each tab, test as much functionality as you can within that tab.

- Checkouts
- Holds
- Recalls (with UseRecalls enabled)
- Claims (with a LOST value defined in ClaimReturnedLostValue)
- Restrictions
- Article Requests (with ArticleRequests preference enabled)
- Clubs (with at least one club defined)
- Relatives' checkouts (patron must have a guarantee linked to their
  account).

Perform the same tests on the patron details page. Note that the
"Charges" tab is shown on the patron details page but not the checkout
page. This is not a change made by this patch.
Comment 11 Owen Leonard 2023-06-02 12:57:18 UTC
(In reply to Sam Lau from comment #9)
> Here is a video of the bug as well

Thanks, I'm not sure why I wasn't seeing it before I'd failed to update all the places where the changed DOM structure needed to be updated in the JavaScript
Comment 12 Sam Lau 2023-06-02 14:41:30 UTC
Created attachment 151984 [details] [review]
Bug 33528: Use template wrapper for tabs: Patron details page

This patch updates the checkout and patron details templates so
that they use the new WRAPPER directive to build tabbed navigation.

The markup for the tab navigation and tab panels is moved into its own
include file since the templates were duplicating essentially the same
code.

To fully test you should have patrons with checkouts, fines, holds,
recalls, and article requests.

Apply the patch and go to the checkout page in the staff interface. With
each tab, test as much functionality as you can within that tab.

- Checkouts
- Holds
- Recalls (with UseRecalls enabled)
- Claims (with a LOST value defined in ClaimReturnedLostValue)
- Restrictions
- Article Requests (with ArticleRequests preference enabled)
- Clubs (with at least one club defined)
- Relatives' checkouts (patron must have a guarantee linked to their
  account).

Perform the same tests on the patron details page. Note that the
"Charges" tab is shown on the patron details page but not the checkout
page. This is not a change made by this patch.

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 13 Katrin Fischer 2023-06-08 19:48:04 UTC
First time I noticed this: when doing actions on the tabs, the page reloads and you lose your 'active' tab. Maybe we could take a look at this sometime. But this is the same without the patch set.
Comment 14 Katrin Fischer 2023-06-08 19:48:35 UTC
Created attachment 152216 [details] [review]
Bug 33528: Use template wrapper for tabs: Patron details page

This patch updates the checkout and patron details templates so
that they use the new WRAPPER directive to build tabbed navigation.

The markup for the tab navigation and tab panels is moved into its own
include file since the templates were duplicating essentially the same
code.

To fully test you should have patrons with checkouts, fines, holds,
recalls, and article requests.

Apply the patch and go to the checkout page in the staff interface. With
each tab, test as much functionality as you can within that tab.

- Checkouts
- Holds
- Recalls (with UseRecalls enabled)
- Claims (with a LOST value defined in ClaimReturnedLostValue)
- Restrictions
- Article Requests (with ArticleRequests preference enabled)
- Clubs (with at least one club defined)
- Relatives' checkouts (patron must have a guarantee linked to their
  account).

Perform the same tests on the patron details page. Note that the
"Charges" tab is shown on the patron details page but not the checkout
page. This is not a change made by this patch.

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Owen Leonard 2023-06-13 11:56:23 UTC
Created attachment 152291 [details] [review]
Bug 33528: Use template wrapper for tabs: Patron details page

This patch updates the checkout and patron details templates so
that they use the new WRAPPER directive to build tabbed navigation.

The markup for the tab navigation and tab panels is moved into its own
include file since the templates were duplicating essentially the same
code.

To fully test you should have patrons with checkouts, fines, holds,
recalls, and article requests.

Apply the patch and go to the checkout page in the staff interface. With
each tab, test as much functionality as you can within that tab.

- Checkouts
- Holds
- Recalls (with UseRecalls enabled)
- Claims (with a LOST value defined in ClaimReturnedLostValue)
- Restrictions
- Article Requests (with ArticleRequests preference enabled)
- Clubs (with at least one club defined)
- Relatives' checkouts (patron must have a guarantee linked to their
  account).

Perform the same tests on the patron details page. Note that the
"Charges" tab is shown on the patron details page but not the checkout
page. This is not a change made by this patch.

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Owen Leonard 2023-06-13 11:56:26 UTC
Created attachment 152292 [details] [review]
Bug 33528: (follow-up) Correct selector for event handler

The event handler for restriction deletion needs to be adjusted
according to the new container ID. Without this change the JavaScript
confirmation message didn't appear and restrictions were deleted without
confirmation.
Comment 17 Tomás Cohen Arazi (tcohen) 2023-06-14 12:24:33 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 18 Tomás Cohen Arazi (tcohen) 2023-06-14 15:47:11 UTC
Created attachment 152344 [details] [review]
Bug 33528: Fix basic_workflow.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Martin Renvoize (ashimema) 2023-07-17 11:13:41 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 20 Martin Renvoize (ashimema) 2023-07-17 11:15:45 UTC
This was a fairly involved backport, but I felt it made sense to aid future backports and conflicts there.
Comment 21 Pedro Amorim 2023-07-18 10:52:24 UTC
Does not apply to 22.11.x. Not pushing.