Bug 33408

Summary: Fetch sysprefs from svc/config/systempreferences
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ERMAssignee: Jonathan Druart <jonathan.druart>
Status: Needs documenting --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: jacob.omara, jonathan.druart, jonathan.field, lucas, martin.renvoize, pedro.amorim, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00
Bug Depends on: 33289, 33402    
Bug Blocks: 33606, 33416    
Attachments: Bug 33408: Fetch sysprefs from svc/config/systempreferences
Bug 33408: Mock ERM sysprefs from cypress tests
Bug 33408: Fetch sysprefs from svc/config/systempreferences
Bug 33408: Mock ERM sysprefs from cypress tests
Bug 33408: Fetch sysprefs from svc/config/systempreferences
Bug 33408: Mock ERM sysprefs from cypress tests
Bug 33408: Extend defaultCommandTimeout for cypress
Bug 33408: Remove obsolete template sys pref code
Bug 33408: Prevent access to erm module by URL if sys pref is disabled
Bug 33408: Remove obsolete tests and template sys pref code
Bug 33408: We need only one this.loaded call
Bug 33408: Fetch sysprefs from svc/config/systempreferences
Bug 33408: Mock ERM sysprefs from cypress tests
Bug 33408: Extend defaultCommandTimeout for cypress
Bug 33408: Prevent access to erm module by URL if sys pref is disabled
Bug 33408: Remove obsolete tests and template sys pref code
Bug 33408: We need only one this.loaded call
Bug 33408: Store sysprefs in a new ERM store
Bug 33408: Store sysprefs in a new ERM store
Bug 33408: Store sysprefs in a new ERM store
Bug 33408: Add storeToRefs to ERMStore
Bug 33408: Move providers into syspref array
Bug 33408: Fetch sysprefs from svc/config/systempreferences
Bug 33408: Mock ERM sysprefs from cypress tests
Bug 33408: Extend defaultCommandTimeout for cypress
Bug 33408: Prevent access to erm module by URL if sys pref is disabled
Bug 33408: Remove obsolete tests and template sys pref code
Bug 33408: We need only one this.loaded call
Bug 33408: Store sysprefs in a new ERM store
Bug 33408: Add storeToRefs to ERMStore
Bug 33408: Move providers into syspref array
Bug 33408: Fix template literal

Description Jonathan Druart 2023-04-04 14:42:32 UTC
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.
Comment 1 Jonathan Druart 2023-04-04 15:04:11 UTC
Created attachment 149131 [details] [review]
Bug 33408: Fetch sysprefs from svc/config/systempreferences

It will be easier to mock/set them from cypress tests.
Comment 2 Jonathan Druart 2023-04-04 15:04:13 UTC
Created attachment 149132 [details] [review]
Bug 33408: Mock ERM sysprefs from cypress tests

Sponsored-by: BULAC - http://www.bulac.fr/
Comment 3 Jonathan Druart 2023-04-04 15:07:26 UTC
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/
Comment 4 Jonathan Druart 2023-04-04 15:07:29 UTC
Created attachment 149134 [details] [review]
Bug 33408: Mock ERM sysprefs from cypress tests

Sponsored-by: BULAC - http://www.bulac.fr/
Comment 5 Jonathan Druart 2023-04-05 07:38:42 UTC
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/
Comment 6 Jonathan Druart 2023-04-05 07:38:44 UTC
Created attachment 149185 [details] [review]
Bug 33408: Mock ERM sysprefs from cypress tests

Sponsored-by: BULAC - http://www.bulac.fr/
Comment 7 Jonathan Druart 2023-04-05 07:44:57 UTC
*** Bug 33370 has been marked as a duplicate of this bug. ***
Comment 8 Jonathan Druart 2023-04-05 08:04:14 UTC
Created attachment 149186 [details] [review]
Bug 33408: Extend defaultCommandTimeout for cypress

Sometimes the initial page load takes more than 4 seconds.
Comment 9 Pedro Amorim 2023-04-05 12:13:09 UTC
Created attachment 149196 [details] [review]
Bug 33408: Remove obsolete template sys pref code
Comment 10 Pedro Amorim 2023-04-05 12:13:11 UTC
Created attachment 149197 [details] [review]
Bug 33408: Prevent access to erm module by URL if sys pref is disabled
Comment 11 Pedro Amorim 2023-04-05 12:14:52 UTC
Created attachment 149198 [details] [review]
Bug 33408: Remove obsolete tests and template sys pref code
Comment 12 Jonathan Druart 2023-04-05 12:27:26 UTC
Created attachment 149200 [details] [review]
Bug 33408: We need only one this.loaded call
Comment 13 Pedro Amorim 2023-04-05 13:04:13 UTC
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>
Comment 14 Pedro Amorim 2023-04-05 13:04:16 UTC
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>
Comment 15 Pedro Amorim 2023-04-05 13:04:18 UTC
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>
Comment 16 Pedro Amorim 2023-04-05 13:04:21 UTC
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>
Comment 17 Pedro Amorim 2023-04-05 13:04:24 UTC
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>
Comment 18 Pedro Amorim 2023-04-05 13:04:27 UTC
Created attachment 149209 [details] [review]
Bug 33408: We need only one this.loaded call

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 19 Jonathan Druart 2023-04-07 08:43:17 UTC
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
Comment 20 Jonathan Druart 2023-04-07 08:45:12 UTC
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.
Comment 21 Pedro Amorim 2023-04-11 11:38:12 UTC
Last patch doesn't apply after bug 33066
Comment 22 Jonathan Druart 2023-04-11 12:25:16 UTC
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
Comment 23 Pedro Amorim 2023-04-12 09:45:09 UTC
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>
Comment 24 Pedro Amorim 2023-04-12 09:45:11 UTC
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>
Comment 25 Pedro Amorim 2023-04-12 09:45:14 UTC
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>
Comment 26 Martin Renvoize 2023-04-12 12:44:14 UTC
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>
Comment 27 Martin Renvoize 2023-04-12 12:44:17 UTC
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>
Comment 28 Martin Renvoize 2023-04-12 12:44:20 UTC
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>
Comment 29 Martin Renvoize 2023-04-12 12:44:22 UTC
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>
Comment 30 Martin Renvoize 2023-04-12 12:44:25 UTC
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>
Comment 31 Martin Renvoize 2023-04-12 12:44:28 UTC
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>
Comment 32 Martin Renvoize 2023-04-12 12:44:31 UTC
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>
Comment 33 Martin Renvoize 2023-04-12 12:44:34 UTC
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>
Comment 34 Martin Renvoize 2023-04-12 12:44:36 UTC
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>
Comment 35 Martin Renvoize 2023-04-12 12:45:54 UTC
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 36 Tomás Cohen Arazi 2023-04-12 14:35:24 UTC
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
Comment 37 Pedro Amorim 2023-04-12 16:32:36 UTC
Created attachment 149570 [details] [review]
Bug 33408: Fix template literal
Comment 38 Pedro Amorim 2023-04-12 16:34:57 UTC
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.
Comment 39 Pedro Amorim 2023-04-12 16:52:01 UTC
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
Comment 40 Tomás Cohen Arazi 2023-04-13 14:49:56 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 41 Jacob O'Mara 2023-04-17 07:27:06 UTC
Enhancement - not backporting to 22.11.x

Nice work everyone!
Comment 42 Lucas Gass 2023-06-15 19:41:09 UTC
(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?