-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
refactor: changed survey report message location and added a new info… #34033
Conversation
…rmative sent state
Thanks for the pull request, @Asespinel! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
openedx/features/survey_report/templates/survey_report/admin_banner.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, lgtm. The wording is clearer now and the banner is in a better location
Thanks for this refactor @Asespinel. The changes you proposed make sense. I have two comments:
|
Thanks for the help @MaferMazu I implemented the fix for your problem and I also added the blank line as requested. Let me know if you need anything else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works as expected. Thanks for the fix @Asespinel.
Can we merge this, or should we wait for comments by @ormsbee?
No need to wait on my review. Thank you. |
@Asespinel 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
Description
This pull request introduces a refactor of the Survey Report management in the Django admin site. The changes include two key modifications aimed at enhancing the clarity and functionality of the admin interface.
refactor: changed survey report banner location and added the sent state #34024
is that we don't touch the models file and we just change the state column to show the "sent" status based on the SurveyReportUpload status code.
Testing
Example: