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

Only show the main error instead of the latest created efile error #5528

Merged

Conversation

arinchoi03
Copy link
Contributor

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

  • Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

  • Hackiest, narrowest solution possible to unblock clients, so they'll see the main error they need to correct which should help resolve the related errors.

How to test?

  • Describe the testing approach taken to verify the changes, including:
    • Unit tests
    • Should test on heroku by creating these efile errors & put client in rejected state
  • Risk Assessment
    • I hate it, but there's supposed to be a long-term solution coming soon, that probably involves some kind of mapping between related issues

if last_efile_errors.where(code: "Form502-01150-010").present?
return last_efile_errors.where(code: "Form502-01150-010").first
end

@submission_to_show&.efile_submission_transitions&.where(to_state: 'rejected')&.last&.efile_errors&.last
Copy link
Contributor Author

@arinchoi03 arinchoi03 Feb 5, 2025

Choose a reason for hiding this comment

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

I tried to do last_efile_errors.last here but this breaks a bunch of specs on return_status_controller_spec.rb...any idea why? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok this is weird, if i run the broken specs one-by-one, they start passing:

rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:1:1:1:4:1:3]' # StateFile::Questions::ReturnStatusController az #edit assignment of various instance variables efile error should expose error when cancelled, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:2:1:1:4:1:2]' # StateFile::Questions::ReturnStatusController id #edit assignment of various instance variables efile error should expose error when waiting, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:3:1:1:4:1:1]' # StateFile::Questions::ReturnStatusController md #edit assignment of various instance variables efile error should expose error when notified_of_rejection, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:3:1:1:4:1:2]' # StateFile::Questions::ReturnStatusController md #edit assignment of various instance variables efile error should expose error when waiting, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:4:1:1:4:1:1]' # StateFile::Questions::ReturnStatusController nc #edit assignment of various instance variables efile error should expose error when notified_of_rejection, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:4:1:1:4:1:3]' # StateFile::Questions::ReturnStatusController nc #edit assignment of various instance variables efile error should expose error when cancelled, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:4:1:5:4:3]' # StateFile::Questions::ReturnStatusController nc #edit rejected auto-wait error shows a generic message when resolution not present
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:5:1:1:4:1:3]' # StateFile::Questions::ReturnStatusController nj #edit assignment of various instance variables efile error should expose error when cancelled, assigns the last efile error attached to the last rejected transition
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:5:1:5:3:1]' # StateFile::Questions::ReturnStatusController nj #edit rejected auto-cancel error shows next steps when present
rspec './spec/controllers/state_file/questions/return_status_controller_spec.rb[1:5:1:5:4:3]' # StateFile::Questions::ReturnStatusController nj #edit rejected auto-wait error shows a generic message when resolution not present

Copy link
Contributor Author

@arinchoi03 arinchoi03 Feb 5, 2025

Choose a reason for hiding this comment

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

@mrotondo @jenny-heath ty for looking at this together! Summary: we didn't figure out exactly why this is happening...but some part of the issue is that the &.last on the efile errors is not returning the order we expect it to be consistently. Going to add this as an issue to address in the long-term story.

Please add notes if you want to the next jira ticket for the long-term solve or here

Copy link

github-actions bot commented Feb 5, 2025

Heroku app: https://gyr-review-app-5528-3a2208354159.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5528 (optionally add --tail)

efile_error: related_efile_error_2,
efile_submission_transition: rejected_transition,
efile_submission_id: efile_submission.id
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are more than 2 related errors for MD, but thought 2 were enough -- lmk if we want to test all of them

Copy link
Contributor

@mrotondo mrotondo left a comment

Choose a reason for hiding this comment

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

Thanks for making this change, and working through the complications!

@arinchoi03 arinchoi03 merged commit 3f04db2 into main Feb 6, 2025
7 checks passed
@arinchoi03 arinchoi03 deleted the FYST-1761-temporary-reject-resolution-for-waiting-clients branch February 6, 2025 22:23
jachan pushed a commit that referenced this pull request Feb 7, 2025
…5528)

* Only show the main error instead of the latest created efile error

* Refactor

* Enforce order by id on efile_errors to get consistent "last" efile_error

* Add comments on the ordering

* Add more comment

* Fix typo

* Update to the correct error code to be the "main" one
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.

3 participants