Bug 33408 - Fetch sysprefs from svc/config/systempreferences
Summary: Fetch sysprefs from svc/config/systempreferences
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
: 33370 (view as bug list)
Depends on: 33289 33402
Blocks: 33606 33416
  Show dependency treegraph
 
Reported: 2023-04-04 14:42 UTC by Jonathan Druart
Modified: 2023-06-15 19:41 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 33408: Fetch sysprefs from svc/config/systempreferences (2.12 KB, patch)
2023-04-04 15:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Mock ERM sysprefs from cypress tests (5.72 KB, patch)
2023-04-04 15:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Fetch sysprefs from svc/config/systempreferences (8.70 KB, patch)
2023-04-04 15:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Mock ERM sysprefs from cypress tests (5.72 KB, patch)
2023-04-04 15:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Fetch sysprefs from svc/config/systempreferences (8.52 KB, patch)
2023-04-05 07:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Mock ERM sysprefs from cypress tests (5.72 KB, patch)
2023-04-05 07:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Extend defaultCommandTimeout for cypress (746 bytes, patch)
2023-04-05 08:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Remove obsolete template sys pref code (3.18 KB, patch)
2023-04-05 12:13 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Prevent access to erm module by URL if sys pref is disabled (6.70 KB, patch)
2023-04-05 12:13 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Remove obsolete tests and template sys pref code (3.19 KB, patch)
2023-04-05 12:14 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: We need only one this.loaded call (1.99 KB, patch)
2023-04-05 12:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Fetch sysprefs from svc/config/systempreferences (8.57 KB, patch)
2023-04-05 13:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Mock ERM sysprefs from cypress tests (5.78 KB, patch)
2023-04-05 13:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Extend defaultCommandTimeout for cypress (806 bytes, patch)
2023-04-05 13:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Prevent access to erm module by URL if sys pref is disabled (6.75 KB, patch)
2023-04-05 13:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Remove obsolete tests and template sys pref code (3.25 KB, patch)
2023-04-05 13:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: We need only one this.loaded call (2.04 KB, patch)
2023-04-05 13:04 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Store sysprefs in a new ERM store (7.11 KB, patch)
2023-04-07 08:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Store sysprefs in a new ERM store (7.14 KB, patch)
2023-04-11 12:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33408: Store sysprefs in a new ERM store (7.20 KB, patch)
2023-04-12 09:45 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Add storeToRefs to ERMStore (1.97 KB, patch)
2023-04-12 09:45 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Move providers into syspref array (5.13 KB, patch)
2023-04-12 09:45 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: Fetch sysprefs from svc/config/systempreferences (8.63 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Mock ERM sysprefs from cypress tests (5.84 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Extend defaultCommandTimeout for cypress (869 bytes, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Prevent access to erm module by URL if sys pref is disabled (6.80 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Remove obsolete tests and template sys pref code (3.31 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: We need only one this.loaded call (2.10 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Store sysprefs in a new ERM store (7.25 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Add storeToRefs to ERMStore (2.03 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Move providers into syspref array (5.18 KB, patch)
2023-04-12 12:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33408: Fix template literal (1.03 KB, patch)
2023-04-12 16:32 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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?