We are retrieving the syspref from the template, we could retrieve them from a route. It will be easier to mock/set them from cypress tests.
Created attachment 149131 [details] [review] Bug 33408: Fetch sysprefs from svc/config/systempreferences It will be easier to mock/set them from cypress tests.
Created attachment 149132 [details] [review] Bug 33408: Mock ERM sysprefs from cypress tests Sponsored-by: BULAC - http://www.bulac.fr/
Created attachment 149133 [details] [review] Bug 33408: Fetch sysprefs from svc/config/systempreferences It will be easier to mock/set them from cypress tests. Sponsored-by: BULAC - http://www.bulac.fr/
Created attachment 149134 [details] [review] Bug 33408: Mock ERM sysprefs from cypress tests Sponsored-by: BULAC - http://www.bulac.fr/
Created attachment 149184 [details] [review] Bug 33408: Fetch sysprefs from svc/config/systempreferences It will be easier to mock/set them from cypress tests. Sponsored-by: BULAC - http://www.bulac.fr/
Created attachment 149185 [details] [review] Bug 33408: Mock ERM sysprefs from cypress tests Sponsored-by: BULAC - http://www.bulac.fr/
*** Bug 33370 has been marked as a duplicate of this bug. ***
Created attachment 149186 [details] [review] Bug 33408: Extend defaultCommandTimeout for cypress Sometimes the initial page load takes more than 4 seconds.
Created attachment 149196 [details] [review] Bug 33408: Remove obsolete template sys pref code
Created attachment 149197 [details] [review] Bug 33408: Prevent access to erm module by URL if sys pref is disabled
Created attachment 149198 [details] [review] Bug 33408: Remove obsolete tests and template sys pref code
Created attachment 149200 [details] [review] Bug 33408: We need only one this.loaded call
Created attachment 149204 [details] [review] Bug 33408: Fetch sysprefs from svc/config/systempreferences It will be easier to mock/set them from cypress tests. Sponsored-by: BULAC - http://www.bulac.fr/ Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149205 [details] [review] Bug 33408: Mock ERM sysprefs from cypress tests Sponsored-by: BULAC - http://www.bulac.fr/ Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149206 [details] [review] Bug 33408: Extend defaultCommandTimeout for cypress Sometimes the initial page load takes more than 4 seconds. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149207 [details] [review] Bug 33408: Prevent access to erm module by URL if sys pref is disabled Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149208 [details] [review] Bug 33408: Remove obsolete tests and template sys pref code Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149209 [details] [review] Bug 33408: We need only one this.loaded call Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149254 [details] [review] Bug 33408: Store sysprefs in a new ERM store We need the sysprefs' values to be shared between the components
There were 2 components using the erm_providers js variable from the .tt We need to store the values to make them reusable from other components. I am not 100% happy with this patch, there is certainly something to improve, for instance we should not need two variables here: + this.ERMStore.providers = providers.value.split(",") + this.providers = this.ERMStore.providers But here is a quick fix to restore the feature.
Last patch doesn't apply after bug 33066
Created attachment 149438 [details] [review] Bug 33408: Store sysprefs in a new ERM store We need the sysprefs' values to be shared between the components
Created attachment 149516 [details] [review] Bug 33408: Store sysprefs in a new ERM store We need the sysprefs' values to be shared between the components Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149517 [details] [review] Bug 33408: Add storeToRefs to ERMStore It makes it so that any changes to the local property is also reflected in the store and other components are made aware of the new value. No need to explicitly change both local property and store property values. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149518 [details] [review] Bug 33408: Move providers into syspref array ERMProviders is a sys pref, place it in sysprefs Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 149535 [details] [review] Bug 33408: Fetch sysprefs from svc/config/systempreferences It will be easier to mock/set them from cypress tests. Sponsored-by: BULAC - http://www.bulac.fr/ Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149536 [details] [review] Bug 33408: Mock ERM sysprefs from cypress tests Sponsored-by: BULAC - http://www.bulac.fr/ Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149537 [details] [review] Bug 33408: Extend defaultCommandTimeout for cypress Sometimes the initial page load takes more than 4 seconds. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149538 [details] [review] Bug 33408: Prevent access to erm module by URL if sys pref is disabled Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149539 [details] [review] Bug 33408: Remove obsolete tests and template sys pref code Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149540 [details] [review] Bug 33408: We need only one this.loaded call Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149541 [details] [review] Bug 33408: Store sysprefs in a new ERM store We need the sysprefs' values to be shared between the components Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149542 [details] [review] Bug 33408: Add storeToRefs to ERMStore It makes it so that any changes to the local property is also reflected in the store and other components are made aware of the new value. No need to explicitly change both local property and store property values. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 149543 [details] [review] Bug 33408: Move providers into syspref array ERMProviders is a sys pref, place it in sysprefs Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This is a nice clear improvement.. QA script squeeks about template literals in ktd, but I thought we'd decided those were OK now? Happy to PQA is someone is happy to verify that template literals are OK now
Comment on attachment 149535 [details] [review] Bug 33408: Fetch sysprefs from svc/config/systempreferences Review of attachment 149535 [details] [review]: ----------------------------------------------------------------- Can you review this? Thanks ::: koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js @@ +10,5 @@ > get sysprefs() { > return { > + get: (variable) => > + this.get({ > + endpoint: `/?pref=${variable}`, FAIL koha-tmpl/intranet-tmpl/prog/js/vue/fetch/system-preferences-api-client.js FAIL ES template literals ES template literals found ( endpoint: `/?pref=${variable}`,) - See bug 24725
Created attachment 149570 [details] [review] Bug 33408: Fix template literal
I think there's something wrong in this patch: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149535 With it applied, if you access http://localhost:8081/cgi-bin/koha/erm/agreements on a new window, it never gets past "Loading", but clicking on the different menu items and going back to agreements, all works fine. It's only with agreements too. This doesn't happen with licenses or any of the others. I'm looking into this but ask if someone else can confirm if they can also reproduce this.
The behaviour I'm experiencing is fixed by bug 33483, more specifically this patch: https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=33483&attachment=149564
Pushed to master for 23.05. Nice work everyone, thanks!
Enhancement - not backporting to 22.11.x Nice work everyone!
(In reply to Jacob O'Mara from comment #41) > Enhancement - not backporting to 22.11.x > > Nice work everyone! It appears like this was backported to 22.11. Can you check?