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

Fix batch determination different forms issue #4304

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sandeepsajan0
Copy link
Member

Fixes #4290

Test Steps

  • Try to perform batch update status action on submissions
  • Try to update some batch determinations(dismissed, accept and more info), all should work fine.

Copy link

sentry-io bot commented Dec 24, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: hypha/apply/determinations/views.py

Function Unhandled Issue
form_valid AttributeError: 'NoneType' object has no attribute 'id' /apply/submissions/{submission_pk}/determination/{pk...
Event Count: 1

Did you find this useful? React with a 👍 or 👎

)
if redirect:
return HttpResponseClientRedirect(redirect.url)
# should redirect
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved should_redirect method directly in here to avoid raising an Error, instead showing the error to the user without performing the batch action.

It is hard to find all the cases at once so I guess it is a better way to avoid 500. We can include the cases to validation as soon as it is reported.

@sandeepsajan0 sandeepsajan0 marked this pull request as ready for review December 24, 2024 07:36
@frjo frjo added Type: Bug Bugs! Things that are broken :-/ Type: Minor Minor change, used in release drafter labels Dec 31, 2024
@frjo

This comment was marked as outdated.

@frjo frjo self-requested a review January 7, 2025 08:49
frjo

This comment was marked as outdated.

@frjo frjo force-pushed the fix/gh-4290-batch-determination-issue branch from 7aea2b5 to 3744288 Compare January 10, 2025 16:47
@frjo frjo force-pushed the fix/gh-4290-batch-determination-issue branch from d5ea79a to 5743656 Compare January 10, 2025 17:00
@@ -317,6 +320,13 @@ def sub_menu_update_status(request: HttpRequest) -> HttpResponse:
else []
)

# hide determination actions if submissions have different forms
if not allow_determination:
determination_actions = ["Dismiss", "Request More Information", "Accept"]
Copy link
Member

Choose a reason for hiding this comment

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

@sandeepsajan0 Will this work with translations?

@frjo frjo added Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Jan 14, 2025
@wes-otf
Copy link
Contributor

wes-otf commented Jan 16, 2025

In testing this definitely was a lot smoother! @sandeepsajan0 I noticed when I selected a couple of lab submissions they all gave me the option to bulk dismiss, and the same with when I selected fund submissions, but combining them removed that option - is this because of the difference in determination forms? if so I think this is ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team Status: Needs testing Tickets that need testing/qa Type: Bug Bugs! Things that are broken :-/ Type: Minor Minor change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Batch determination breaks with different form submissions
3 participants