feat: diamond specific FAP spreadsheet columns#1656
Open
GrantDLS wants to merge 8 commits into
Open
Conversation
… to avoid needing to use positional arguments.
GrantDLS
changed the title
GrantDLS
changed the title
GrantDLS
changed the title
TCMeldrum
reviewed
Jul 22, 2026
Contributor
There was a problem hiding this comment.
Do you ever run the cypress tests in DEPENDENCY_CONFIG === 'dls'? if not i don't think we need this file?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
We want to include some different fields in the FAP spreadsheet download, so this change adds in the new fields and renames some others. It uses dependency injection to introduce this functionality so that existing configurations are unaffected.
Motivation and Context
The
getDataRowfunction was changed from positional parameters to a singleFapDataRowInputobject because the function is selected through dependency injection and different facility implementations require different inputs.The facility-specific implementations require different subsets of proposal data, and the positional API became fragile as new values such as the instrument ID were introduced. A named object prevents argument-order errors, provides a stable interface across implementations, and allows each facility to consume only the fields it requires.
The reason this worked before is because the function was being called with 14 parameters. The default implementation only needed 11 params and so would just ignore the additional 3. Following this same approach meant that the DLS implementation wasn't able to only accept 12 params - it needed to accept the extra three from the STFC implementation, plus the extra instrument ID that was needed.
How Has This Been Tested
Extra cypress fixtures have been added to test the inclusion of the new columns and the renamed ones.