Created attachment 171556 [details] [review]
Bug 37930: Add a method to initialise the AV store

This patch adds a method to initialise the authorised values in a vue app. It will automatically load authorised values into the app and store them in the authorised values store based on the entries in a matrix file. The intent behind this is to make adding authorised values into a Vue app much simpler and more DRY, especially when creating a new vue app. You will just need to add your app into the avMatrix.js file and add a call to the initialisation method in your app.

Created attachment 171557 [details] [review]
Bug 37930: Update the ERM and Preservation apps

Created attachment 171558 [details] [review]
Bug 37930: Update cypress tests

Test plan:
1) Run the cypress tests for ERM and Preservation
2) All tests should still pass

This is an idea for discussion - the main intent is to make the handling of authorised values DRY across all Vue apps and to make it much easier to inject them into both existing and new apps

The avMatrix file controls which authorised values are injected into the app so to use an additional value category in your app you would just need to add a line into that file under the relevant module

For new apps, you would just add an entry into the matrix file and call the initialisation method somewhere in your home component and the front loading of all the authorised values would be handled for you

I think we should keep one file per app. Otherwise we will end up with a big ugly and hard-to-maintain authorisedValues/avMatrix.js.

Why not simply have the av list in the store of the app (eg. stores/erm.js)?

That would mean we could remove useAVStore and use the mainStore instead. We would retrieve the AVs from the config:
  this.config.avs.get("my_av")
  this.config.avs.get_lib_from_av("my_av", value)

Also for discussion :)

Created attachment 171628 [details] [review]
Bug 37930: [DO NOT PUSH] Alternate concept

Alternative patch taking into account feedback in comment 5 on the bug. This patch removes the authorised values store and migrates the methods to the main store. Authorised values are now added into the module specific store and replaced on module load with the actual values

To test I've added a console log of the values in the store to the AgreementsList component so apply all patches on the bug and then navigate to the agreements table in ERM. The module will throw errors as I haven't been through and changed all the instances where we would need to swap the AV store for the ERM store (there are a lot of instances and this is just a proof of concept for discussion for now). Check that the logged values are as expected. If you have the Vue devtools you'll be able to see the store values and check them directly in there

(In reply to Jonathan Druart from comment #5)
> I think we should keep one file per app. Otherwise we will end up with a big
> ugly and hard-to-maintain authorisedValues/avMatrix.js.
> 
> Why not simply have the av list in the store of the app (eg. stores/erm.js)?
> 
> That would mean we could remove useAVStore and use the mainStore instead. We
> would retrieve the AVs from the config:
>   this.config.avs.get("my_av")
>   this.config.avs.get_lib_from_av("my_av", value)
> 
> Also for discussion :)

I like the idea of using the module specific store so the new patch uses that methodology. I've opted not to include them in the config object as we overwrite that when we fetch the config and that wipes the existing authorised values we would have fetched

Have a look at the patch above and see what you think - I've not changed all of the places we would need to remove the AV store yet as this may need changes and that would take time to keep repeating.

That seems better to me.

A couple of notes:
* "loaded" is not called when the pref is off (and so "loading..." is displayed on the screen)

* it would be nice to still have the av related functions (get_lib_from_av, map_av_dt_filter, loadAuthorisedValues) in a dedicated file. They were isolated and it was good. We should try to keep that in mind, to keep stores/main.js short and clean. But I would not consider it blocker.