https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153914&action=edit Bug 34417: Allow breadcrumbs to accept dynamic values This patch is an example of allowing a breadcrumb to accept a dynamic value such as an agreement or license name. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #3 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #2)

There are two things I would like to note here: 1. It does not feel correct to not invalidate the parameter when the view changed. We could end up with a name from the previous view. It's highlighted especially now, as we don't have all the routes modified.

Agreed, I'll add a line to reset it after each render

2. It does not feel optimal to have to modify each component. It would be nicer if we could have the value set in the route definition. For instance we could have: title: $__("Show {agreement.name}") Also we would not need the 'dynamic' flag and title could stay a string. I have no idea how this could be implemented however.

This would be preferable but I couldn't think of a way of doing this as the routes are currently generated statically when the application loads. I've just been re-reviewing and I wonder if we could pass a function as the title property...? Then maybe we could emit something from within the API client rather than each component which that function could pick up. That way the data passing is centralised between the api client and the navigation store. I'll have a play around and see if I can get something to work -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #5 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Hopefully this applies correctly following the split out from 33169 - let me know if not and I can rebase -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153958|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 153960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153960&action=edit Bug 34417: Move logic to http client This patch moves the logic for setting dynamic breadcrumb values to the http client to avoid modifying all components that need dynamic values. This patch also fixes the warning about discarding invalid params when navigating Test plan: Create some agreements/licenses/titles/packages and navigate through the ERM module especially to the "show" pages and the "edit" pages for those respective items. On these pages there should be a descriptive breadcrumb using either the name or title of the item. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Interesting, but having this code in _fetchJSON seems wrong. It will be called too many times. Also it's not possible to have "Edit {agreement.name}", it is what we do in other places. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #11 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- I'm looking at adding a class to each route to determine whether its an agreement/license etc (programatically, not manually in erm.js). I think I'll move the logic out of _fetchJSON into the get() request as that should be the only time we need dynamic data in the breadcrumb. Within that I can then use the added route class to determine which requests should actually set parameters (e.g. licenses shouldn't trigger it when an agreement route is called) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 154263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154263&action=edit Bug 34417: [ALTERNATIVE PATCH] Set entity's name from the component itself I would have preferred to not modify the component to pass what we have fetched, but I think it's cleaner than hacking the low level fetch function. This is just a POC and work only on the "show agreement" view -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #15 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Passing the data from the component is for sure the cleanest method. Setting the breadcrumb from the get() function was the best method I could find for avoiding doing that but I think it will be harder to maintain if we get a lot more edge cases/more complicated routes -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 --- Comment #17 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- I think that is the best option of the two, far less likely to introduce regressions -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34417 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38201 -- You are receiving this mail because: You are watching all bug changes.