Bug 37213 - Improve breadcrumbs in rotating collections
Summary: Improve breadcrumbs in rotating collections
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Eric Garcia
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-27 14:59 UTC by Lucas Gass
Modified: 2024-07-07 04:39 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 37213: Improved breadcrumbs in rotating collections (1.80 KB, patch)
2024-06-27 19:35 UTC, Eric Garcia
Details | Diff | Splinter Review
Bug 37213: Improved breadcrumbs in rotating collections (1.84 KB, patch)
2024-06-27 19:46 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 37213: (follow-up) Remove stray </em>, make title logic consistent with breadcrumbs (2.47 KB, patch)
2024-06-28 11:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37213: Improved breadcrumbs in rotating collections (1.89 KB, patch)
2024-07-07 04:36 UTC, David Nind
Details | Diff | Splinter Review
Bug 37213: (follow-up) Remove stray </em>, make title logic consistent with breadcrumbs (2.51 KB, patch)
2024-07-07 04:36 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2024-06-27 14:59:50 UTC
1. Tools > Rotating collections > New collection 
2. Add a title and description 
3. After adding the new collection notice the breadcrumb isn't right:

 Tools > Rotating collections > Collection " > Edit collection
Comment 1 Eric Garcia 2024-06-27 19:35:36 UTC Comment hidden (obsolete)
Comment 2 Sam Lau 2024-06-27 19:46:53 UTC
Created attachment 168216 [details] [review]
Bug 37213: Improved breadcrumbs in rotating collections

To test:
1. Tools -> Rotating collections -> New collections
2. Add a title and description
3. After adding the new collection notice the breadcrumb isn't right
4. Apply patch
5. Repeat steps 1 and 2
6. After adding the new collection notice the breadcrumb is improved.

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 3 Marcel de Rooy 2024-06-28 09:39:16 UTC
Added Assignee now. Please fill yourself next time :)
Comment 4 Marcel de Rooy 2024-06-28 09:51:02 UTC
This looks a bit odd. I would not expect created or deleted to pop up in the breadcrumbs. If something failed, we should have a dialog alert somewhere else.

Furthermore there seems to be an issue with:
            [% WRAPPER breadcrumb_item %]
                <a href="/cgi-bin/koha/rotating_collections/addItems.pl?colId=[% editColId | uri %]">[% tx("Collection '{collection_title}'", { collection_title = editColTitle }) | html %]</em></a>
            [% END %]

The quoted empty string comes from there. Should the script pass a correct op and record id here instead? If you delete a collection, probably the op should change to new. If you added or updated, we need the record id.

Note also that I see an </em> without an opening <em> ?
Comment 5 Owen Leonard 2024-06-28 11:23:18 UTC
Created attachment 168232 [details] [review]
Bug 37213: (follow-up) Remove stray </em>, make title logic consistent with breadcrumbs
Comment 6 Owen Leonard 2024-06-28 11:26:12 UTC
(In reply to Marcel de Rooy from comment #4)
> This looks a bit odd. I would not expect created or deleted to pop up in the
> breadcrumbs. If something failed, we should have a dialog alert somewhere
> else.

We do have a dialog alert. The thing which is inconsistent about this page is that there is a "creation success/failure" stage at all. Usually we would redirect to the list of collections in either case, adding a dialog if there was an error. 

I think these changes are logical given the unusual workflow of having a "success" page.


> The quoted empty string comes from there. Should the script pass a correct
> op and record id here instead? If you delete a collection, probably the op
> should change to new. If you added or updated, we need the record id.
> 
> Note also that I see an </em> without an opening <em> ?

These issues are all unrelated to the changes in the original patc. I've submitted a follow-up to correct them.
Comment 7 David Nind 2024-07-07 04:36:20 UTC
Created attachment 168575 [details] [review]
Bug 37213: Improved breadcrumbs in rotating collections

To test:
1. Tools -> Rotating collections -> New collections
2. Add a title and description
3. After adding the new collection notice the breadcrumb isn't right
4. Apply patch
5. Repeat steps 1 and 2
6. After adding the new collection notice the breadcrumb is improved.

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2024-07-07 04:36:23 UTC
Created attachment 168576 [details] [review]
Bug 37213: (follow-up) Remove stray </em>, make title logic consistent with breadcrumbs

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2024-07-07 04:39:44 UTC
Not related to this bug, but I'm not sure that I understand the difference between Tools > Rotating collections and Cataloging > Automation > Stock rotation (when the StockRotation system preference is enabled).

Having similar names is a bit confusing.