perf: optimise generic template copying

[ellen-wright]

Description

Closes: UserOfficeProject/issue-tracker#1193
This PR optimises copying generic templates from a previous proposal.
Before optimisation the query was taking on average 23 seconds to load the proposals and generic templates the user has access to.
A user should have access to any proposal:

• They are a member of
• They are a reviewer of
• That uses an instrument they are a scientist on
• That uses an instrument that they are the instrument contact for
• They are FAP chair on
• They are FAP secretary on
• They are a visitor on
  Following the criteria set out in ProposalAuthorization.ts User Officers and Instrument Scientists have access to all generic templates.

This PR creates a new database view and a new query to reduce the time to load these proposals to 200ms on average.

Motivation and Context

This change is needed to reduce unnecessary data being retrieved when a user attempts to copy a previous generic template. Currently every generic template is returned by the getGenericTemplates query and filtering which ones a user can see is done in ProposalAuthorization.ts. Therefore potentially >1000 unnecessary generic templates were being retrieved each time.

How Has This Been Tested

This has been tested manually, I made some changes to frontend tests.
Comparison in time from previous query to new query:
This is for a user with a reasonable amount of previous generic templates, the time will increase if user is also a scientist on an instrument or if they qualify more of the above criteria, but the increases in performance are still significant.

getGenericTemplates (s)	getGenericTemplatesForCopy (s)
23.02	0.135
23.02	0.139
23.04	0.128
23.01	0.131
23.67	0.122
23.61	0.114
23.70	0.123
23.84	0.121

Changes

A new database view generic_templates_view has been added.

• This reduces the complexity of the new query as all database table joins are done within the view.
• It lists all generic templates along with it's associated, FAP reviewers, FAP chairs, FAP secretaries, visitors, instrument scientists, instrument contacts, along with generic template data including creator_id.

A new query getGenericTemplatesForCopy has been created.

• This query is called when a user tries to retrieved their previous generic templates, it returns all generic templates they have access to according to the previously listed criteria.
• As User Officers and Instrument Scientists have access to all generic templates the new query is only used for the User role.

Changes to the frontend tests to fall in line with how generic templates are queried for copying

• Adds creator Id to created generic templates in the tests

Tests included/Docs Updated?

• I have added tests to cover my changes.
• All relevant doc has been updated

[ACLay]

I realise it's already been done a lot in the UOP, but I don't like that we add extra query endpoints to the backend for what feels like the same data. I really think we should be able to do this with just an extra filter option like investigatorId in the current getGenericTemplates query.

While a user should have access to a proposal (and thus it's generic template answers) in all those listed situations, I think that list is the top level case for proposal access, and not relevant for copying generic templates, which are only done by a user when filling in a questionnaire, and in that role/situation I think it'd be fine to have the query just join and filter on the PI and co investigator ID's

[jekabs-karklins]

Hi, Ellen, nice PR.
I left two quick comments, please let me know.

And one additional thing. I think I saw that we are mixing the two names "copy" and "clone", i.e. cloneGenericTemplate

Maybe it is worth to use one name everywhere.

[jekabs-karklins]

nitpick: Maybe we can call this "user_id" because it is a number. "agent" elsewhere in the code is of type "UserWithRole". I think.

[ellen-wright]

I realise it's already been done a lot in the UOP, but I don't like that we add extra query endpoints to the backend for what feels like the same data. I really think we should be able to do this with just an extra filter option like investigatorId in the current getGenericTemplates query.

While a user should have access to a proposal (and thus it's generic template answers) in all those listed situations, I think that list is the top level case for proposal access, and not relevant for copying generic templates, which are only done by a user when filling in a questionnaire, and in that role/situation I think it'd be fine to have the query just join and filter on the PI and co investigator ID's

I realise it's already been done a lot in the UOP, but I don't like that we add extra query endpoints to the backend for what feels like the same data. I really think we should be able to do this with just an extra filter option like investigatorId in the current getGenericTemplates query.

While a user should have access to a proposal (and thus it's generic template answers) in all those listed situations, I think that list is the top level case for proposal access, and not relevant for copying generic templates, which are only done by a user when filling in a questionnaire, and in that role/situation I think it'd be fine to have the query just join and filter on the PI and co investigator ID's

I see what you're saying about this new query returning basically the same data but I think the thinking behind it was that these 2 queries are now doing quite different things with where they are called and how they filter. getGenericTemplates is called in quite a few places and having a new query cleanly separates the different functionality they represent.
I have tried to make them into one query and reducing the number of criteria we are filtering by but after a checking with Emma I think the correct functionality is that everyone in the list should have access to these generic templates. And combining them reduced the improvements in efficiency I was getting.

[ellen-wright]

Hi, Ellen, nice PR. I left two quick comments, please let me know.

And one additional thing. I think I saw that we are mixing the two names "copy" and "clone", i.e. cloneGenericTemplate

Maybe it is worth to use one name everywhere.

I think we are using clone when duplicating a generic template from the same proposal and copy when it is from a different proposal, it might get confusing which functionality something is referring to if we use the same name for both of these

[ACLay]

It just doesn't feel... the most elegant? in the spirit of graphql? to me, to have multiple queries that return the same type of object, have filter parameters that could co-exist, and have one of those queries only exist to be used in a single place. I still think one would be better for flexibilities sake, allowing for potential mixing of arguments in the future, and I'm not sure why the query needs provision for the various fap, scientist & instrument roles to select templates for copying when copying is only ever done by a proposer when filling their template with their data, but if this is the only way to get the performance boost we need, then I guess we have to go with it. We're after speed as a metric, not vibes.