Bug 38941 - Convert the acquisitions menu to a Vue island
Summary: Convert the acquisitions menu to a Vue island
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Matt Blenkinsop
QA Contact: Jonathan Druart
URL:
Keywords: release-notes-needed
Depends on: 37911 38930
Blocks: 38010
  Show dependency treegraph
 
Reported: 2025-01-21 16:45 UTC by Matt Blenkinsop
Modified: 2025-09-16 03:10 UTC (History)
11 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:
25.05.00
Circulation function:


Attachments
Bug 38941: Convert the acquisitions menu to a Vue web component (23.19 KB, patch)
2025-01-21 17:22 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.40 KB, patch)
2025-01-21 17:22 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Convert the acquisitions menu to a Vue web component (23.04 KB, patch)
2025-02-18 14:03 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.40 KB, patch)
2025-02-18 14:03 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.40 KB, patch)
2025-04-25 08:50 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Convert the acquisitions menu to a Vue web component (21.25 KB, patch)
2025-04-25 08:51 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.40 KB, patch)
2025-04-25 08:51 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Rebase for follow-up to bug 37911 (1.17 KB, patch)
2025-04-28 15:39 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Convert the acquisitions menu to a Vue web component (21.29 KB, patch)
2025-04-28 15:44 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.45 KB, patch)
2025-04-28 15:44 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 38941: Rebase for follow-up to bug 37911 (1.22 KB, patch)
2025-04-28 15:44 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.20 KB, patch)
2025-04-30 10:03 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38941: Convert the acquisitions menu to a Vue web component (21.38 KB, patch)
2025-04-30 20:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38941: Add a cypress test (2.33 KB, patch)
2025-04-30 20:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38941: Rebase for follow-up to bug 37911 (1.30 KB, patch)
2025-04-30 20:55 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 2025-01-21 16:45:16 UTC
We should convert the left menus to Vue islands so that the components can be used both in Koha and in Vue apps where needed
Comment 1 Matt Blenkinsop 2025-01-21 17:22:00 UTC
Created attachment 176877 [details] [review]
Bug 38941: Convert the acquisitions menu to a Vue web component

This is an example patch for creating a Vue island component that can be used throughout Koha. The component can then also be used in the work in Bug 38010 to migrate Vendors to Vue. This removes duplication of code by having a .inc file as well as a vue component to maintain

Test plan:
1) Apply all patches including dependencies
2) Bug 37911 requires a Vue upgrade so you will need to run yarn install, then run a yarn build
3) Navigate to the acquisitions home page
4) The left menu should appear as before
5) Switch on the EDIFACT syspref - the acq menu should now display the edifact related options
6) Try logging in as a new patron with reduced permissions, the menu should update correctly according to the permissions
Comment 2 Matt Blenkinsop 2025-01-21 17:22:02 UTC
Created attachment 176878 [details] [review]
Bug 38941: Add a cypress test
Comment 3 Matt Blenkinsop 2025-01-21 17:23:26 UTC
This is a POC - interested to hear any ideas for improvement. I considered using the NavigationItem component to render the links but it changes the HTML structure slightly and is largely dependent on the Vue router
Comment 4 Matt Blenkinsop 2025-02-18 14:03:30 UTC
Created attachment 178243 [details] [review]
Bug 38941: Convert the acquisitions menu to a Vue web component

This is an example patch for creating a Vue island component that can be used throughout Koha. The component can then also be used in the work in Bug 38010 to migrate Vendors to Vue. This removes duplication of code by having a .inc file as well as a vue component to maintain

Test plan:
1) Apply all patches including dependencies
2) Bug 37911 requires a Vue upgrade so you will need to run yarn install, then run a yarn build
3) Navigate to the acquisitions home page
4) The left menu should appear as before
5) Switch on the EDIFACT syspref - the acq menu should now display the edifact related options
6) Try logging in as a new patron with reduced permissions, the menu should update correctly according to the permissions
Comment 5 Matt Blenkinsop 2025-02-18 14:03:32 UTC
Created attachment 178244 [details] [review]
Bug 38941: Add a cypress test
Comment 6 Janne Seppänen 2025-02-21 13:07:52 UTC
Tried testing on ByWater Solutions sandbox and on a local git build, but both end up with an error and the left menu doesn't appear at all.

Seems like the code is looking for a module that's not there?

Uncaught Error: Cannot find module '../stores/permissions'
    webpackMissingModule https://staff-bugiton.sandbox.bywatersolutions.com/intranet-tmpl/prog/js/vue/dist/islands_24.1200012.js:16487
    <anonymous> https://staff-bugiton.sandbox.bywatersolutions.com/intranet-tmpl/prog/js/vue/dist/islands_24.1200012.js:16487
    <anonymous> https://staff-bugiton.sandbox.bywatersolutions.com/intranet-tmpl/prog/js/vue/dist/islands_24.1200012.js:16774
islands_24.1200012.js:16487:50
Comment 7 Katrin Fischer 2025-03-13 16:51:51 UTC
Hi Janne, this bug has 2 dependencies that are not resolved yet. The sandboxes are not very clear when there is a conflict on applying the patches. Maybe have a look at the git log if you can.
Comment 8 Matt Blenkinsop 2025-04-25 08:50:13 UTC
Created attachment 181491 [details] [review]
Bug 38941: Add a cypress test
Comment 9 Matt Blenkinsop 2025-04-25 08:51:00 UTC
Created attachment 181492 [details] [review]
Bug 38941: Convert the acquisitions menu to a Vue web component

This is an example patch for creating a Vue island component that can be used throughout Koha. The component can then also be used in the work in Bug 38010 to migrate Vendors to Vue. This removes duplication of code by having a .inc file as well as a vue component to maintain

Test plan:
1) Apply all patches including dependencies
2) Bug 37911 requires a Vue upgrade so you will need to run yarn install, then run a yarn build
3) Navigate to the acquisitions home page
4) The left menu should appear as before
5) Switch on the EDIFACT syspref - the acq menu should now display the edifact related options
6) Try logging in as a new patron with reduced permissions, the menu should update correctly according to the permissions
Comment 10 Matt Blenkinsop 2025-04-25 08:51:04 UTC
Created attachment 181493 [details] [review]
Bug 38941: Add a cypress test
Comment 11 Matt Blenkinsop 2025-04-28 15:39:07 UTC
Created attachment 181626 [details] [review]
Bug 38941: Rebase for follow-up to bug 37911

Bug 37911 added a follow-up that removed the original entry point from rspack to use the esm.js files
Comment 12 Biblibre Sandboxes 2025-04-28 15:44:41 UTC
Created attachment 181627 [details] [review]
Bug 38941: Convert the acquisitions menu to a Vue web component

This is an example patch for creating a Vue island component that can be used throughout Koha. The component can then also be used in the work in Bug 38010 to migrate Vendors to Vue. This removes duplication of code by having a .inc file as well as a vue component to maintain

Test plan:
1) Apply all patches including dependencies
2) Bug 37911 requires a Vue upgrade so you will need to run yarn install, then run a yarn build
3) Navigate to the acquisitions home page
4) The left menu should appear as before
5) Switch on the EDIFACT syspref - the acq menu should now display the edifact related options
6) Try logging in as a new patron with reduced permissions, the menu should update correctly according to the permissions

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Comment 13 Biblibre Sandboxes 2025-04-28 15:44:45 UTC
Created attachment 181628 [details] [review]
Bug 38941: Add a cypress test

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Comment 14 Biblibre Sandboxes 2025-04-28 15:44:48 UTC
Created attachment 181629 [details] [review]
Bug 38941: Rebase for follow-up to bug 37911

Bug 37911 added a follow-up that removed the original entry point from rspack to use the esm.js files

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Comment 15 Matt Blenkinsop 2025-04-30 10:03:36 UTC
Created attachment 181711 [details] [review]
Bug 38941: Add a cypress test
Comment 16 Jonathan Druart 2025-04-30 20:55:10 UTC
Created attachment 181724 [details] [review]
Bug 38941: Convert the acquisitions menu to a Vue web component

This is an example patch for creating a Vue island component that can be used throughout Koha. The component can then also be used in the work in Bug 38010 to migrate Vendors to Vue. This removes duplication of code by having a .inc file as well as a vue component to maintain

Test plan:
1) Apply all patches including dependencies
2) Bug 37911 requires a Vue upgrade so you will need to run yarn install, then run a yarn build
3) Navigate to the acquisitions home page
4) The left menu should appear as before
5) Switch on the EDIFACT syspref - the acq menu should now display the edifact related options
6) Try logging in as a new patron with reduced permissions, the menu should update correctly according to the permissions

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2025-04-30 20:55:14 UTC
Created attachment 181725 [details] [review]
Bug 38941: Add a cypress test

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2025-04-30 20:55:17 UTC
Created attachment 181726 [details] [review]
Bug 38941: Rebase for follow-up to bug 37911

Bug 37911 added a follow-up that removed the original entry point from rspack to use the esm.js files

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Katrin Fischer 2025-05-02 13:44:08 UTC
1) QA script

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc
   FAIL	  filters
		missing_filter at line 25 ([% SET islands = Asset.js("js/vue/dist/islands.esm.js").match('(src="([^"]+)")').1 %] <script src="[% islands %]" type="module">)
		missing_filter at line 28 (    import { hydrate } from "[% islands %]";)

I fixed it with the automated script in a separate commit. Please check!

2) Question about permission check 

We have this here, which appears to be a bit of a double up on first glance - do we need this in order to over both TT and vue cases where the menu is used?

+    suggestionscreate="[% CAN_user_suggestions_suggestions_create | html %]"
+    suggestionsmanage="[% CAN_user_suggestions_suggestions_manage | html %]"
+    suggestionsdelete="[% CAN_user_suggestions_suggestions_delete | html %]"

And then:

+                v-if="
+                    suggestionscreate ||
+                    suggestionsmanage ||
+                    suggestionsdelete ||
+                    isUserPermitted(
+                        'CAN_user_suggestions_suggestions_create'
+                    ) ||
+                    isUserPermitted(
+                        'CAN_user_suggestions_suggestions_manage'
+                    ) ||
+                    isUserPermitted('CAN_user_suggestions_suggestions_delete')
+                "
+            >
Comment 20 Katrin Fischer 2025-05-02 13:48:28 UTC
1) - I can see now that this code is removed by 38010. So dropping my follow-up.
Comment 21 Katrin Fischer 2025-05-02 15:24:12 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 22 Fridolin Somers 2025-08-04 14:43:27 UTC
Enhancement not pushed to 24.11.x
Comment 23 David Cook 2025-09-11 23:43:57 UTC
Little question... I notice this bug depends on bug 38930, but then we're using Template::Toolkit to set attributes mapped to permissions on the acquisitions-menu web component. 

Looking again at the template, it looks like some parts of the template use the permission store from bug 38930 and some use the attributes/properties for the web component? 

Seems like it's only half-implemented here?
Comment 24 Matt Blenkinsop 2025-09-12 08:21:43 UTC
Hi David, the component that is used in the custom element can be used either as a Vue custom element in template toolkit or as a component in a Vue application. When used in a Vue app we get access to the pinia stores (permissionsStore) and can use the method provided by the permissionsStore to check permissions. Outside of a vue app however, we don't get access to a populated permissionsStore so we need to be able to accept the permissions from the template toolkit variables as component props
Comment 25 David Cook 2025-09-14 23:00:58 UTC
(In reply to Matt Blenkinsop from comment #24)
> the component that is used in the custom element can be used
> either as a Vue custom element in template toolkit or as a component in a
> Vue application. When used in a Vue app we get access to the pinia stores
> (permissionsStore) and can use the method provided by the permissionsStore
> to check permissions. Outside of a vue app however, we don't get access to a
> populated permissionsStore so we need to be able to accept the permissions
> from the template toolkit variables as component props

Looking back at the patch again, I see that it does support both methodologies for all cases. On first read, I thought it was using one methodology in one place and the other methodology in other places. My bad!

That said, can you explain things more? In the case of acquisitions menu, it looks like you do have access to the pinia stores, but that it's just not being populated? It also looks like it gets mounted as a single component Vue app in the end? Looking at bug 38930, it looks like the permissionsStore is not used anywhere currently? There's the ERM example which shows an ERM-specific implementation for using the permissionsStore, but that seems suboptimal. 

When I first heard of bug 38930, I thought it was going to introduce a core API endpoint for fetching current user details and then put the permissions in the permissionsStore for use by Vue components. Even outside of Vue, I reckon having an API endpoint to fetch that information would be useful.
Comment 26 Matt Blenkinsop 2025-09-15 08:43:47 UTC
 
> That said, can you explain things more? In the case of acquisitions menu, it
> looks like you do have access to the pinia stores, but that it's just not
> being populated? It also looks like it gets mounted as a single component
> Vue app in the end? Looking at bug 38930, it looks like the permissionsStore
> is not used anywhere currently? There's the ERM example which shows an
> ERM-specific implementation for using the permissionsStore, but that seems
> suboptimal.
> 
> When I first heard of bug 38930, I thought it was going to introduce a core
> API endpoint for fetching current user details and then put the permissions
> in the permissionsStore for use by Vue components. Even outside of Vue, I
> reckon having an API endpoint to fetch that information would be useful.

Its currently being used by the Vendors Vue module - when the acquisitions menu is loaded as part of the vendors vue app we have access to the permissions store as it is populated by the module's config endpoint and the menu is just used as a generic vue component rather than a custom element. When it is used as a custom element, the permissionsStore is technically available but isn't populated as there is no specific vue app with a config endpoint to populate from. A future enhancement could certainly be to update the permissions store to self-populate and remove the need for the TT variables
Comment 27 David Cook 2025-09-16 03:10:22 UTC
(In reply to Matt Blenkinsop from comment #26)
> Its currently being used by the Vendors Vue module - 

Ah I see it now. Both the dependency chain in Bugzilla and the import of ./Islands/AcquisitionsMenu.vue in koha-tmpl/intranet-tmpl/prog/js/vue/components/LeftMenu.vue. 

> when the acquisitions
> menu is loaded as part of the vendors vue app we have access to the
> permissions store as it is populated by the module's config endpoint and the
> menu is just used as a generic vue component rather than a custom element.

I see what you mean when looking at /cgi-bin/koha/acqui/acqui-home.pl vs /cgi-bin/koha/acquisition/vendors?supplier=

> When it is used as a custom element, the permissionsStore is technically
> available but isn't populated as there is no specific vue app with a config
> endpoint to populate from. A future enhancement could certainly be to update
> the permissions store to self-populate and remove the need for the TT
> variables

I reckon that would be a much more DRY option for sure. 

Anyway, cool beans. Thanks for explaining all that.