Bug 40656 - bookings/list.tt needs to be refactored
Summary: bookings/list.tt needs to be refactored
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Paul Derscheid
QA Contact: Testopia
URL:
Keywords:
Depends on: 40296
Blocks: 36789
  Show dependency treegraph
 
Reported: 2025-08-15 10:57 UTC by Paul Derscheid
Modified: 2025-08-15 15:49 UTC (History)
5 users (show)

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


Attachments
Bug 40656: Refactor bookings/list.tt to use better patterns and improve code organization (17.40 KB, patch)
2025-08-15 15:24 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 40656: (follow-up) Create reusable additional filters library for DataTables (18.67 KB, patch)
2025-08-15 15:24 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 40656: (follow-up) Correct inverted logic for column visibility (2.18 KB, patch)
2025-08-15 15:30 UTC, Paul Derscheid
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2025-08-15 10:57:54 UTC
When QAing Bug 40296 and looking at Bug 36789, I felt like it was time to clean some of the more wip bits up.

This includes:
- Inlined CSS
- Incomprehensible logic for the table filters (sorry, that was on me)
- QA complaints from Bug 36789
Comment 1 Paul Derscheid 2025-08-15 15:24:02 UTC
Created attachment 185468 [details] [review]
Bug 40656: Refactor bookings/list.tt to use better patterns and improve code organization

This patch aims to improve bookings/list.tt:
- Converting filter attributes to boolean data attributes (presence-based)
- Creating a reusable table-filters.inc for filter controls
- Moving inline styles to proper SCSS files for better caching and separation of concerns
- Fixing a runtime error when cancelling bookings from the timeline
- Improving JavaScript readability with helper functions
- Addressing the QA complaints from Bug 36789:
    - Cleaning up the long lines for the button elements by moving them into template literals
    - Using the format method instead of direct interpolation
    - Using escape_str for all values received from the API.

Test plan:
1. Apply the patch and compile css
2. Navigate to a biblio record with bookings (Catalog > Search > View record > Bookings)
3. Verify the filter buttons (Exclude expired/cancelled) appear correctly
4. Click each filter button and verify:
 - The button text toggles between "Include" and "Exclude"
 - The table updates to show/hide the appropriate bookings
 - The status column appears when any items are filtered
5. In the timeline view:
 - Verify cancelled bookings show with a strikethrough pattern
 - Try to cancel a booking by clicking the X on a timeline item
 - Confirm the cancel modal opens without JavaScript errors
6. Check that the table actions (Edit, Cancel) work correctly
7. Verify no inline styles remain in the page source
8. Confirm the filter styles are properly loaded from compiled CSS
Comment 2 Paul Derscheid 2025-08-15 15:24:04 UTC
Created attachment 185469 [details] [review]
Bug 40656: (follow-up) Create reusable additional filters library for DataTables

This follow-up patch extracts the filter functionality into a reusable library:
- Create AdditionalFilters.js library with chainable API for filter
functionality
- Rename table-filters.inc to additional-filters.inc for clarity
- Refactor to use explicit element IDs instead of interpolation for
better greppability (filter-expired vs filter-${id})
- Include proper cleanup methods (destroy) for memory management

The library provides a fluent API:
AdditionalFilters.init(['filter-expired', 'filter-cancelled'])
    .onChange((filters, { anyFiltersApplied }) => { ... })
    .build({ status: ({ filters, isNotApplied }) => ... });

Test plan remains identical except for applying both patches on step 1.
Comment 3 Paul Derscheid 2025-08-15 15:30:10 UTC
Created attachment 185470 [details] [review]
Bug 40656: (follow-up) Correct inverted logic for column visibility