Description
Matt Blenkinsop
2025-01-21 16:45:16 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 Created attachment 176878 [details] [review] Bug 38941: Add a cypress test 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 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 Created attachment 178244 [details] [review] Bug 38941: Add a cypress test 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 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. Created attachment 181491 [details] [review] Bug 38941: Add a cypress test 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 Created attachment 181493 [details] [review] Bug 38941: Add a cypress test 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 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> Created attachment 181628 [details] [review] Bug 38941: Add a cypress test Signed-off-by: Michaela <michaela.sieber@kit.edu> 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> Created attachment 181711 [details] [review] Bug 38941: Add a cypress test 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> 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> 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> 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') + " + > 1) - I can see now that this code is removed by 38010. So dropping my follow-up. Pushed for 25.05! Well done everyone, thank you! |