Use investigationFacilityCycles to determine the facility cycles an Investigation belongs to

[kennethnym]

Description

This PR changes the way facility cycles are calculated for a given investigation. Instead of using start dates and end dates of an investigation to determine what facility cycle it falls into, the investigationFacilityCycles field of the Investigation entity provided by the backend can be used . It is an array of facility cycles that the investigations belong to. For building URLs to investigations, the first investigation facility cycle in the array is always used.

I created 3 new functions in datagateway-common called build<Investigation/Dataset/Datafile>Url that each constructs a URL to the given investigation/dataset/datafile. They either return the URL as a string, or null if the URL cannot be created due to missing info (for example, the investigation object provided does not have the investigationFacilityCycles field)

Edit 2/2/2023
I also created a constant called FACILITY_NAME in datagateway-common app.types.tsx that defines all the known facility names, i.e. isis and dls, to reduce the excessive use of magic strings in the code.

Testing instructions

• Review code
• Check Actions build
• Review changes to test coverage

Agile board tracking

Closes #1411

[codecov]

Codecov Report

Patch coverage: 96.08% and project coverage change: -0.22 ⚠️

Comparison is base (1154804) 96.55% compared to head (6249c8e) 96.33%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1415      +/-   ##
===========================================
- Coverage    96.55%   96.33%   -0.22%     
===========================================
  Files          159      161       +2     
  Lines         7252     6934     -318     
  Branches      2296     2146     -150     
===========================================
- Hits          7002     6680     -322     
- Misses         228      233       +5     
+ Partials        22       21       -1     

Flag	Coverage Δ
common	95.63% <86.00%> (-0.32%)	⬇️
dataview	96.96% <100.00%> (+0.06%)	⬆️
download	96.73% <ø> (+0.01%)	⬆️
search	96.50% <100.00%> (-0.69%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...etailsPanels/dls/datasetDetailsPanel.component.tsx	100.00% <ø> (ø)
packages/datagateway-common/src/testData.ts	0.00% <0.00%> (ø)
packages/datagateway-common/src/urlBuilders.ts	97.29% <97.29%> (ø)
...ages/datagateway-common/src/api/facilityCycles.tsx	100.00% <100.00%> (ø)
...ages/datagateway-common/src/api/investigations.tsx	100.00% <100.00%> (ø)
packages/datagateway-common/src/app.types.tsx	100.00% <100.00%> (ø)
...ateway-dataview/src/page/doiRedirect.component.tsx	100.00% <100.00%> (ø)
...card/isis/isisInvestigationsCardView.component.tsx	98.43% <100.00%> (+0.10%)	⬆️
...s/table/isis/isisInvestigationsTable.component.tsx	97.05% <100.00%> (+0.18%)	⬆️
...src/views/table/isis/isisMyDataTable.component.tsx	98.46% <100.00%> (+2.07%)	⬆️
... and 5 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[louise-davies]

This influences the Lucene search PR as that needs the URLs changing

The backend generator script needs to ensure it creates these links

Maybe deploy this on ISIS dev so ISIS can try it out?

[kennethnym]

New change: created a constant called FACILITY_NAME in datagateway-common app.types.tsx that defines all the known facility names, i.e. isis and dls, to reduce the excessive usage of magic strings in the code.

[louise-davies]

I believe we also need to refactor the facility cycles endpoint - as currently we're querying the special dg-api endpoint that does the date calculation logic (and it's causing errors - e.g. on ISIS dev cycle 20_1 appears under ALF but has no investigations when you click on it - my way below correctly doesn't return cycle 20_1). Instead, we should query the facilitycycles endpoint directly using the new investigationFacilityCycles relation. I believe this query works?

/facilitycycles?where={"investigationFacilityCycles.investigation.investigationInstruments.instrument.id":{"eq":INSTRUMENT_ID}}&distinct=["name", "startDate", "endDate"]

This uses the relation to find facility cycles that have investigations that are on the instrument with id INSTRUMENT_ID. Distinct is needed as otherwise it returns duplicate cycles for every cycle with a unique investigation with the matching instrument id (and in fact the original ISIS query under the hood uses DISTINCT as well in dg-api) - the three distinct fields are used as those are the values we rely on in the UI (theoretically we lose the description in card view but I think this is always just ISIS Cycle in practice so not very helpful anyway).

Do feel free to play about and see if you can find a different way to query, this is just the first way I found that gives the correct results.

[louise-davies]

All the changes you made look good and it works well in dataview - I've noticed that my standard search on ISIS had quite a few investigations with no cycles - so I'll raise this with ISIS

[louise-davies]

Ignore me - I just realised they're investigations with no data - which is exactly the case we've already raised that have no cycles: https://github.com/ral-facilities/isis-icat-ingestion/issues/76#issuecomment-1266751672

I'm happy for this to go in then once we can get the e2e tests working once that work is done, so we can put this in blocked for now.

[louise-davies]

LGTM - just would appreciate @kennethnym looking at the last few commits of mine to check they're fine :)

[kennethnym]

Looks good, thanks for fixing the tests :)