Skip to content

refactor: update fap reviews and meetings tables - #1892

Open
TCMeldrum wants to merge 17 commits into
developfrom
update-fapproposal-and-meeting-table
Open

TCMeldrum wants to merge 17 commits into
developfrom
update-fapproposal-and-meeting-table

Conversation

@TCMeldrum

@TCMeldrum TCMeldrum commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Description

This PR refactors and updates the FAP reviews and meetings tables to improve the data structure and relations (what).

Motivation and Context

The changes are necessary for better data integrity, optimizing the database structure, and improving the performance of data retrieval processes. This also making the link between the fap_proposal and fap_meetings tables more explicted and not based on the instrument as (at least at ISIS) want to be able to set a different instrument in the meeting than has been requested.

I have move the time allocation to the meeting table as it part of the final meeting form not part of the fap proposal. I have also updated the ui to reflect this.

Changes

  1. Introduced new columns recommended_time_allocation and fap_proposal_id in the fap_meeting_decisions table and updated them based on existing data.
  2. Added new columns meeting_status, meeting_notes, and meeting_score in the fap_meeting_decisions table.
  3. Dropped unnecessary columns recommended_time_allocation and instrument_has_proposals_id from the fap_proposals table.
  4. Updated the FapDataSource interface and its mock implementation to reflect the changes in the database schema (how).

How Has This Been Tested?

Fixes Jira Issue

https://jira.ess.eu//browse/

Depends On

Tests included/Docs Updated?

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

@zacharyjhankin
zacharyjhankin self-requested a review October 16, 2026 10:00
@zacharyjhankin

Copy link
Copy Markdown
Collaborator

Hi I just want to say that this is an excellent pull request. This change will be a big improvement long term for us. I can assume that you would want to have the same change to the fap_reviews table where the PK will involve the fap_proposal_id?

@TCMeldrum
TCMeldrum marked this pull request as ready for review October 22, 2026 09:32
@TCMeldrum
TCMeldrum requested a review from a team as a code owner October 22, 2026 09:32
@TCMeldrum
TCMeldrum requested review from Scott-James-Hurley and removed request for a team October 22, 2026 09:32
>
{getFapCodeById(fmd.fapId)} - {t('FAP')} Meeting decision
(Instrument: {getInstrumentNameById(fmd.instrumentId)})
(Instrument: {getInstrumentNameById(fmd.recommendedInstrumentId)})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not against changing fmd.instrumentId to fmd.recommendedInstrumentId but I really think it would be easier to do that seperately in a different PR. This one can be the DB changes, and do that one later

@TCMeldrum

Copy link
Copy Markdown
Contributor Author

I have updated the fmd table to be recommend instrument id. Its a just a rename with no other effects so hope thats ok to keep in.

@yoganandaness
yoganandaness self-requested a review October 28, 2026 13:28

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants