Bug 32932 - Re-structure Vue router-links to use "name" instead of urls
Summary: Re-structure Vue router-links to use "name" instead of urls
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Matt Blenkinsop
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 32983
Blocks: 33169 33417 33941
  Show dependency treegraph
 
Reported: 2023-02-09 16:08 UTC by Matt Blenkinsop
Modified: 2023-07-05 13:05 UTC (History)
7 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:
23.11.00,23.05.01,22.11.07


Attachments
Bug_32932: Update router to use object references (19.53 KB, patch)
2023-02-09 16:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug_32932: Update router to use object references (19.55 KB, patch)
2023-02-09 16:32 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug_32932: Update router to use object references (20.71 KB, patch)
2023-02-14 13:59 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug_32932: Update router to use object references - agreements (20.72 KB, patch)
2023-02-14 14:01 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for licences (6.72 KB, patch)
2023-02-14 14:03 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for eholdings (35.64 KB, patch)
2023-02-14 14:05 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update ermmain to use router name property (1.25 KB, patch)
2023-02-14 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug_32932: Update router to use object references - agreements (20.70 KB, patch)
2023-02-20 14:55 UTC, Jacob O'Mara
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for licences (6.70 KB, patch)
2023-02-20 14:55 UTC, Jacob O'Mara
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for eholdings (35.62 KB, patch)
2023-02-20 14:55 UTC, Jacob O'Mara
Details | Diff | Splinter Review
Bug 32932: Update ermmain to use router name property (1.29 KB, patch)
2023-02-20 14:55 UTC, Jacob O'Mara
Details | Diff | Splinter Review
Bug_32932: Update router to use object references - agreements (22.57 KB, patch)
2023-02-22 10:47 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for licences (6.90 KB, patch)
2023-02-22 10:47 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for eholdings (39.36 KB, patch)
2023-02-22 10:47 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update ermmain to use router name property (1.82 KB, patch)
2023-02-22 10:47 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: (rebase) Rebased on top of bug 32991 (5.15 KB, patch)
2023-03-13 12:08 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update router to use object references - agreements (19.48 KB, patch)
2023-03-22 14:13 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for licences (5.37 KB, patch)
2023-03-22 14:13 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for eholdings (37.02 KB, patch)
2023-03-22 14:13 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update ermmain to use router name property (4.27 KB, patch)
2023-03-22 14:13 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use object references - agreements (19.54 KB, patch)
2023-03-22 14:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for licences (5.42 KB, patch)
2023-03-22 14:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use name property for eholdings (37.07 KB, patch)
2023-03-22 14:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update ermmain to use router name property (4.33 KB, patch)
2023-03-22 14:49 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: (follow-up) Fix copy/paste bug in local titles form add and fix bad link definition in Main.vue (3.43 KB, patch)
2023-04-05 13:04 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: (follow-up) Fix copy/paste bug in local titles form add and fix bad link definition in Main.vue (13.49 KB, patch)
2023-04-14 19:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use object references - agreements (55.03 KB, patch)
2023-04-14 20:34 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Update router to use object references - agreements (55.04 KB, patch)
2023-04-20 11:26 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32932: Update router to use object references - agreements (55.08 KB, patch)
2023-04-20 11:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32932: Update router to use object references - agreements (55.13 KB, patch)
2023-05-22 13:30 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 32932: Fix resource_id param for EHoldingsLocalResourcesShow (1.39 KB, patch)
2023-06-09 07:38 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 Matt Blenkinsop 2023-02-09 16:08:06 UTC
Currently the Vue router is set up with urls assigned to components with no "name" property. This means that in the components, where a router-link tag is used the full URL has been written out again. In the event that a url needs to be changed in the router, the url will also have to be changed in every component that uses that route. 
By using the "name" property we can avoid this and make maintenance of the router much more efficient.
Comment 1 Matt Blenkinsop 2023-02-09 16:21:26 UTC
Created attachment 146439 [details] [review]
Bug_32932: Update router to use object references

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 2 Matt Blenkinsop 2023-02-09 16:32:29 UTC
Created attachment 146442 [details] [review]
Bug_32932: Update router to use object references

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 3 Matt Blenkinsop 2023-02-14 13:59:16 UTC
Created attachment 146616 [details] [review]
Bug_32932: Update router to use object references

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 4 Matt Blenkinsop 2023-02-14 14:01:45 UTC
Created attachment 146617 [details] [review]
Bug_32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 5 Matt Blenkinsop 2023-02-14 14:03:43 UTC
Created attachment 146618 [details] [review]
Bug 32932: Update router to use name property for licences

As per previous commit for agreements
Comment 6 Matt Blenkinsop 2023-02-14 14:05:07 UTC
Created attachment 146624 [details] [review]
Bug 32932: Update router to use name property for eholdings

As per previous commit for agreements
Comment 7 Matt Blenkinsop 2023-02-14 14:06:15 UTC
Created attachment 146625 [details] [review]
Bug 32932: Update ermmain to use router name property

Updating the router links in the homepage as per previous commits
Comment 8 Jacob O'Mara 2023-02-20 14:55:13 UTC
Created attachment 146965 [details] [review]
Bug_32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 9 Jacob O'Mara 2023-02-20 14:55:16 UTC
Created attachment 146966 [details] [review]
Bug 32932: Update router to use name property for licences

As per previous commit for agreements
Comment 10 Jacob O'Mara 2023-02-20 14:55:19 UTC
Created attachment 146967 [details] [review]
Bug 32932: Update router to use name property for eholdings

As per previous commit for agreements
Comment 11 Jacob O'Mara 2023-02-20 14:55:22 UTC
Created attachment 146968 [details] [review]
Bug 32932: Update ermmain to use router name property

Updating the router links in the homepage as per previous commits

Signed-off-by: Jacob O'Mara <jacob.omara@ptfs-europe.com>
Comment 12 Matt Blenkinsop 2023-02-21 09:28:59 UTC
Will need rebasing once bug 32983 and its dependency tree are pushed to master as routes.js has moved/been renamed. I'll apply the dependency tree and rebase, then re-submit once master is up to date with current ERM refactoring work
Comment 13 Matt Blenkinsop 2023-02-22 10:47:04 UTC
Created attachment 147119 [details] [review]
Bug_32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 14 Matt Blenkinsop 2023-02-22 10:47:07 UTC
Created attachment 147120 [details] [review]
Bug 32932: Update router to use name property for licences

As per previous commit for agreements
Comment 15 Matt Blenkinsop 2023-02-22 10:47:10 UTC
Created attachment 147121 [details] [review]
Bug 32932: Update router to use name property for eholdings

As per previous commit for agreements
Comment 16 Matt Blenkinsop 2023-02-22 10:47:12 UTC
Created attachment 147122 [details] [review]
Bug 32932: Update ermmain to use router name property

Updating the router links in the homepage as per previous commits
Comment 17 Matt Blenkinsop 2023-02-22 10:47:59 UTC
REBASED: Now depends on 32983 to reflect current refactoring work. Test plan as before
Comment 18 Agustín Moyano 2023-03-10 18:46:35 UTC
With the new dialog implementation there are no more '*Delete.vue' files.
Comment 19 Matt Blenkinsop 2023-03-13 12:08:32 UTC
Created attachment 148115 [details] [review]
Bug 32932: (rebase) Rebased on top of bug 32991

This patch rebases on top of changes made in bug 32991 and also fixes an
incorrect router reference in EHoldingsLocalTitlesFormAdd
Comment 20 Agustín Moyano 2023-03-21 17:46:02 UTC
Patch still doesn't apply:

14:43 $ git bz apply 32932

Bug 32932 - Re-structure Vue router-links to use "name" instead of urls

147119 - Bug_32932: Update router to use object references - agreements
147120 - Bug 32932: Update router to use name property for licences
147121 - Bug 32932: Update router to use name property for eholdings
147122 - Bug 32932: Update ermmain to use router name property
148115 - Bug 32932: (rebase) Rebased on top of bug 32991

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug_32932: Update router to use object references - agreements
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsFormAdd.vue).
error: could not build fake ancestor
Patch failed at 0001 Bug_32932: Update router to use object references - agreements
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug32932-Update-router-to-use-object-references----8WDpli.patch
Comment 21 Agustín Moyano 2023-03-21 18:18:49 UTC
I'll try to reapply your patches and make them work
Comment 22 Agustín Moyano 2023-03-22 14:13:22 UTC
Created attachment 148551 [details] [review]
Bug 32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings
Comment 23 Agustín Moyano 2023-03-22 14:13:25 UTC
Created attachment 148552 [details] [review]
Bug 32932: Update router to use name property for licences

As per previous commit for agreements
Comment 24 Agustín Moyano 2023-03-22 14:13:29 UTC
Created attachment 148553 [details] [review]
Bug 32932: Update router to use name property for eholdings

As per previous commit for agreements
Comment 25 Agustín Moyano 2023-03-22 14:13:32 UTC
Created attachment 148554 [details] [review]
Bug 32932: Update ermmain to use router name property

Updating the router links in the homepage as per previous commits
Comment 26 Agustín Moyano 2023-03-22 14:49:42 UTC
Created attachment 148558 [details] [review]
Bug 32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 27 Agustín Moyano 2023-03-22 14:49:45 UTC
Created attachment 148559 [details] [review]
Bug 32932: Update router to use name property for licences

As per previous commit for agreements

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 28 Agustín Moyano 2023-03-22 14:49:49 UTC
Created attachment 148560 [details] [review]
Bug 32932: Update router to use name property for eholdings

As per previous commit for agreements

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 29 Agustín Moyano 2023-03-22 14:49:52 UTC
Created attachment 148561 [details] [review]
Bug 32932: Update ermmain to use router name property

Updating the router links in the homepage as per previous commits

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 30 Jonathan Druart 2023-03-23 10:12:34 UTC
-                        this.$router.push(
-                            "/cgi-bin/koha/erm/eholdings/local/titles"
-                        )
+                        this.$router.push({
+                            name: "EHoldingsLocalPackagesList",
+                        })

You are replacing titles with packages here.
Comment 31 Jonathan Druart 2023-03-23 10:15:12 UTC
I am getting a blank page on /cgi-bin/koha/erm/eholdings/local/titles/2

Uncaught Error: No match for
 {"name":"EHoldingslocalPackagesList","params":{}}

"local" vs "Local"
Comment 32 Agustín Moyano 2023-04-05 13:04:32 UTC
Created attachment 149210 [details] [review]
Bug 32932: (follow-up) Fix copy/paste bug in local titles form add and fix bad link definition in Main.vue
Comment 33 Agustín Moyano 2023-04-14 19:21:58 UTC
Created attachment 149707 [details] [review]
Bug 32932: (follow-up) Fix copy/paste bug in local titles form add and fix bad link definition in Main.vue
Comment 34 Agustín Moyano 2023-04-14 20:34:59 UTC
Created attachment 149709 [details] [review]
Bug 32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 35 Agustín Moyano 2023-04-14 20:36:14 UTC
squashed commits for rebase to apply
Comment 36 Matt Blenkinsop 2023-04-20 11:26:21 UTC
Needs another rebase
Comment 37 Matt Blenkinsop 2023-04-20 11:26:44 UTC
Created attachment 149934 [details] [review]
Bug 32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 38 Martin Renvoize 2023-04-20 11:30:22 UTC
Created attachment 149935 [details] [review]
Bug 32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2023-04-20 11:32:57 UTC
Functionality all tests out good, QA scripts are happy :)

Passing QA
Comment 40 Agustín Moyano 2023-05-22 13:30:17 UTC
Created attachment 151529 [details] [review]
Bug 32932: Update router to use object references - agreements

This patch updates the Vue files for ERM to use router object links rather than urls - this will make it much simpler to maintain the router if urls need to be updated. This first patch only updates the files in the agreements section of ERM as a first step to introduce the topic for discussion.

Test plan:
1) Apply patch
2) Navigate to the Agreements section of ERM and click on different navigation links/buttons
3) The links/buttons should all work as normal
4) Review the code to see that the component files now refer back to the router using objects rather than url strings

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Tomás Cohen Arazi 2023-06-06 19:20:40 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 42 Jonathan Druart 2023-06-07 07:46:43 UTC
I think we need a test to prevent regressions.
Comment 43 Matt Blenkinsop 2023-06-07 13:58:32 UTC
The current test suite does a lot of navigation using .click() events - this should cover the router functionality for those routes/links. Do we just need a test that tests the routes that aren't currently included in existing cypress tests or are you thinking of a different testing method?
Comment 44 Jonathan Druart 2023-06-07 14:26:08 UTC
I was more thinking: how do we make sure future changes won't introduce non-named URLs in ERM or even other vue files?
Comment 45 David Cook 2023-06-08 01:00:13 UTC
(In reply to Jonathan Druart from comment #44)
> I was more thinking: how do we make sure future changes won't introduce
> non-named URLs in ERM or even other vue files?

Might be worth seeing if there's a way of using ESLint to check?
Comment 46 Matt Blenkinsop 2023-06-08 08:27:30 UTC
ESLint could work if we create a custom rule, or we'd need to add something into the pre-commit hook to check Vue files for urls
Comment 47 Jonathan Druart 2023-06-09 07:35:34 UTC
This patch makes Titles_spec.ts fail:
  "ReferenceError: resource_id is not defined" in "Show title".

Can you have a look please?
Comment 48 Jonathan Druart 2023-06-09 07:38:16 UTC
Created attachment 152231 [details] [review]
Bug 32932: Fix resource_id param for EHoldingsLocalResourcesShow

Caught by cypress tests  "ReferenceError: resource_id is not defined" in "Show title".
Comment 49 Jonathan Druart 2023-06-09 07:38:29 UTC
(In reply to Jonathan Druart from comment #47)
> This patch makes Titles_spec.ts fail:
>   "ReferenceError: resource_id is not defined" in "Show title".
> 
> Can you have a look please?

Don't worry, I fixed it!
Comment 50 Matt Blenkinsop 2023-06-12 11:43:04 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 51 Tomás Cohen Arazi 2023-07-05 13:05:43 UTC
The follow-up has been pushed to master, 23.05 and 22.11. Removing the 'additional_work_needed' flag, and closing.