[zacharyjhankin]

Description

This PR fixes the bug below. that when you switch the assigned FAP of an instrument the database doesn't update and the new FAP starts editing the old record of the old FAP. This PR makes the key in the fap_meeting_decisions table a composite primary key between the isntrument, fap and proposalID.

Motivation and Context

When you take a proposal and change what FAP panel is assigned to each instrument then the DB does not update the entries in fap_meeting_decisions to change the ID of the FAP panel. This means the right panel can update the wrong record.

Replication steps.

Make a call with two FAP panels
Make a proposal
Assign proposal to both instruments
assign propsoal instrument 1 to fap panel 1
fill in fap meeting form
check out fap_meeting_decisions table
change proposal instrument 1 to fap panel 2
edit fap meeting form
check out fap_meeting_decisions now wrong panel stored.

How Has This Been Tested

Manual testing

Fixes

Changes

Depends on

Tests included/Docs Updated?

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

[jkramya]

Changes are tested locally.

[TCMeldrum]

I know im bit a late but im going to update the primary key for the Meeting table in a small rejig of the fapProposal and fapMeeting table: #1892. Happy for this to go in if you need it sooner than I can get that ready.

[zacharyjhankin]

No worries, we only need to fix this in the next 3 weeks. You can get your code in first @TCMeldrum

[jekabs-karklins]

@zacharyjhankin I was looking at this late evening yesterday and I spotted some issues, please hold with merging this, I'll gat back to this as soon as I can, but essntially after this change a proposal and instrument can have two rows in fap_meeting_decisions (one per FAP) . The review_data view still joins on proposal_pk and instrument_id only (see 0214_FixReviewDataDuplicate.sql, lines 56-57), so it would return the proposal twice.

So we need recreate the view and add AND fmd.fap_id = fp.fap_id line

DROP VIEW IF EXISTS review_data;
			CREATE VIEW review_data AS
			SELECT
			    proposal.proposal_pk,
			    proposal.proposal_id,
			    proposal.title,
			    proposal.instrument_name,
			    proposal.availability_time,
			    proposal.time_allocation,
			    proposal.fap_id,
			    proposal.rank_order,
			    proposal.call_id,
			    proposal.proposer_id,
			    proposal.instrument_id,
			    proposal.fap_time_allocation,
			    proposal.questionary_id,
			    grade.avg AS average_grade,
			    proposal.public_comment AS comment
			FROM (
			    SELECT
			        fp.proposal_pk,
			        p.proposal_id,
			        p.title,
			        i.name AS instrument_name,
			        chi.availability_time,
			        tr.time_allocation,
			        f.fap_id,
			        fmd.rank_order,
			        c.call_id,
			        p.proposer_id,
			        i.instrument_id,
			        fp.fap_time_allocation,
			        p.questionary_id,
			        tr.public_comment
			    FROM fap_proposals fp
			    JOIN faps f ON f.fap_id = fp.fap_id
			    JOIN call c ON c.call_id = fp.call_id
			    JOIN proposals p ON p.proposal_pk = fp.proposal_pk
			    JOIN technical_review tr ON tr.proposal_pk = p.proposal_pk
			        AND tr.instrument_id = fp.instrument_id
			    JOIN call_has_instruments chi ON chi.instrument_id = fp.instrument_id
			        AND chi.call_id = c.call_id
			    JOIN instruments i ON i.instrument_id = chi.instrument_id
			    LEFT JOIN fap_meeting_decisions fmd ON fmd.proposal_pk = p.proposal_pk
			        AND fmd.instrument_id = i.instrument_id
			        AND fmd.fap_id = fp.fap_id
			    LEFT JOIN workflow_has_statuses whs ON whs.workflow_status_id = p.workflow_status_id
			    WHERE whs.status_id <> 'EXPIRED'
			    AND whs.status_id <> 'DRAFT'
			) proposal
			        LEFT JOIN ( SELECT fr.proposal_pk,
			                avg(CASE 
			                WHEN fr.grade ~ '^\d+(\.\d+)\?$' THEN fr.grade::double precision
			                ELSE NULL
			            END) AS avg
			    FROM fap_proposals fp
			    JOIN fap_reviews fr ON fr.proposal_pk = fp.proposal_pk
			    GROUP BY fr.proposal_pk
			) grade ON grade.proposal_pk = proposal.proposal_pk;
	```

[jekabs-karklins]

Hi @zacharyjhankin, sorry for pasting this AI analysis (slightly edited) block directly but what I inspect in the code seems to capture the issue precisely

---

---

---

What was wrong from the beginning

Each proposal has one "meeting decision" record per instrument. This is where the panel stores its ranking, recommendation and comments. When a user officer moves a proposal to a different panel, the system updates the assignment but never clears out the old panel's decision. The new panel then opens the proposal and ends up editing the old panel's record, and the system still files it under the old panel. That's the bug reported in the PR.

What this change tries to do

It gives each panel its own decision record. When the new panel saves, it gets a fresh record filed under the right panel instead of taking over the old one. For the reported scenario this works: the new panel's decision is now stored correctly.

What looks more broken now

The old panel's decision still isn't removed, so a moved proposal now has two decisions for the same instrument, one from each panel. Several parts of the system were built assuming there is only ever one, and they don't check which panel it belongs to. Places where this will look broken:

• Review overviews and reports: a moved proposal can appear twice, or show the old panel's recommendation.
• Submitting the panel's decisions: the old panel's leftover comments can make a proposal count as "complete" when the new panel hasn't finished it.
• The "all panel meetings submitted" automation: for a moved proposal it may never fire, so anything that depends on it won't happen.

So before, the problem was one wrong record. Now it's a stray extra record that confuses several other features. These effects come from reading the code and haven't been tested yet. On the dev system no proposal has been moved this way, so there's nothing to observe there.

The Simpler fix

When a proposal moves to a different panel, delete the old panel's decision for that instrument. Every proposal keeps exactly one decision per instrument, the new panel starts fresh, and nothing else needs to change.

---

---

---

Let me know if you have thought about it

[zacharyjhankin]

Hi thanks @jekabs-karklins

I think the main thing that the AI missed is that the main problem is with the database architecture, and this pull request doesn't address the issue. @TCMeldrum has a wonderful draft PR, that will improve this area, so I think its best to close this one, and wait for that to be merged before changing anything.