Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EWM7751 replaced save tab of reduction with pop up, removed some noisy warnings #482

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

walshmm
Copy link
Collaborator

@walshmm walshmm commented Oct 23, 2024

Description of work

This exposes the workflow completion message to the WorkflowBuilder to enable an implemented workflow to display a custom message upon completion.
This was then utilized to replace the Save step of the ReductionWorkflow (which does nothing) with a simple pop up that accomplishes the same goal: Informing the user that redcution has completed successfully.

This turns off a particularly noisy in-actionable warning message too.

To test

Run Reduction with run number 59039, observe that it completes.
Ensure the reduction results are in the workspace list and are also persisted to disk.

Link to EWM item

EWM#7751

Verification

  • the author has read the EWM story and acceptance critera
  • the reviewer has read the EWM story and acceptance criteria
  • the reviewer certifies the acceptance criteria below reflect the criteria in EWM

Acceptance Criteria

This list is for ease of reference, and does not replace reading the EWM story as part of the review. Verify this list matches the EWM story before reviewing.

  • The new desired solution is to skip the final step (The Reduction Save View) on the reduction workflow and instead simply drop an a prompt informing the user that Reduction had completed successfully
  • The user no longer has the opportunity to create workspaces that will accidentally be deleted at the end of the Reduction Workflow (i.e. painstakingly created masks)

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.53%. Comparing base (205b119) to head (eeffd7c).
Report is 4 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next     #482   +/-   ##
=======================================
  Coverage   96.53%   96.53%           
=======================================
  Files          63       63           
  Lines        4503     4503           
=======================================
  Hits         4347     4347           
  Misses        156      156           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@walshmm walshmm changed the title EQM7751 replaced save tab of reduction with pop up, removed some noisy warnings EWM7751 replaced save tab of reduction with pop up, removed some noisy warnings Oct 23, 2024
Comment on lines +10 to +15
startLambda=None,
iterateLambda=None,
resetLambda=None,
cancelLambda=None,
parent=None,
completionMessageLambda=lambda: Config["ui.default.workflow.completionMessage"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I feel like I could reduce the number of lambdas by just passing the WorkflowImplementer itself.

@walshmm walshmm force-pushed the ewm7751_replace_reduction_save_view_w_popup branch from dc2f1d5 to eeffd7c Compare October 24, 2024 18:50
Copy link
Collaborator

@dlcaballero16 dlcaballero16 left a comment

Choose a reason for hiding this comment

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

Tested and reduction works as it should.

@walshmm walshmm merged commit fee6f7e into next Oct 24, 2024
8 checks passed
@walshmm walshmm deleted the ewm7751_replace_reduction_save_view_w_popup branch October 24, 2024 19:50
rboston628 pushed a commit that referenced this pull request Oct 25, 2024
…y warnings (#482)

* replaced save tab of reduction with pop up, removed some noisy warnings

* add new config props to test yml

* removed comment, updated completion text to use old informative blob

* integration tests now expect workflows to finish with qmessagebox.information
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.

2 participants