Skip to content

Bug/multiple research updates#113

Merged
git-voo merged 4 commits intomainfrom
bug/multiple-research-updates
May 6, 2025
Merged

Bug/multiple research updates#113
git-voo merged 4 commits intomainfrom
bug/multiple-research-updates

Conversation

@Jaywin2099
Copy link
Contributor

@Jaywin2099 Jaywin2099 commented Apr 19, 2025

Fixes #109

What was changed:

Right now, only the threshold is sent to the backend when the submit button is presssed in the ResearcherToolbar component. This PR adds support for the movingAverageFactor variable as well. Tests to verify the correct functionality of this feature were added as well.
console output shows the backend receiving new data

Why was this changed:

In order for the researcher to change the MAF and threshold parameters.

How was it changed:

Adding support for MAF into the sendThreshold function required changing the format from expecting only the threshold to any object.

In order to reduce redundancy and websocket payloads, flags were set to watch for threshold and MAF updates, so that when the submit button is pressed, only updated parameters would be sent to the backend. This was done by creating an empty object and adding whichever variables were flagged as changed from the previous button press.

A few tests were added to make sure only the updated variables were added to the payload, and the existing test, which verified the correct values sent, were expanded to test MAF in addition to threshold values.

@Jaywin2099 Jaywin2099 self-assigned this Apr 19, 2025
@Jaywin2099 Jaywin2099 requested a review from git-voo as a code owner April 19, 2025 22:06
@Jaywin2099 Jaywin2099 linked an issue Apr 19, 2025 that may be closed by this pull request
3 tasks
git-voo
git-voo previously approved these changes Apr 28, 2025
Copy link
Collaborator

@git-voo git-voo left a comment

Choose a reason for hiding this comment

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

This is great, the server receives single and multiple updates as we want. I'd request a change in the alerts to make sure it shows what was updated instead of just "threshold..." at all times. If you are unable to setup personalized alert for this you can use a generic one for the scope of this PR

const [sendMAF, setSendMAF] = useState(false);

function activateToast() {
threshold_toast.current.show({ severity: "success", summary: "Submitted", detail: "Threshold successfully submitted!" });
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will always alert "Threshold successfully submitted!" for all all updates including ONLY MA updates and MA with Threshold update

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Jaywin2099 You can merge this in as soon as you adjust the alert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@git-voo sounds good. The toast now notifies the user of whatever variable was changed, and it warns the user if none were. Merging is still blocked for me, pending your code review.

@git-voo git-voo merged commit 4590f6c into main May 6, 2025
4 checks passed
@git-voo git-voo deleted the bug/multiple-research-updates branch May 6, 2025 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable Single and Multiple Updates from Researcher Toolbar

2 participants