-
Notifications
You must be signed in to change notification settings - Fork 7
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: forms did not redirect to same page if sent from alias #20
Conversation
Reviewer's Guide by SourceryThis pull request fixes a bug where forms did not redirect to the same page when submitted from an alias. The fix involves updating the redirect behavior to explicitly redirect to the same page using the Sequence diagram for form submission and redirection flowsequenceDiagram
participant User as User
participant Form as Form Plugin
participant Server as Server
User->>Form: Submits form
Form->>Server: POST request
Note over Server: Process form data
Server-->>User: Redirect to SAME_PAGE_REDIRECT
Note over User: User stays on same page
Note right of Server: Previously redirected to
Note right of Server: placeholder.page which
Note right of Server: didn't work with aliases
Class diagram showing CMSAjaxBase plugin changesclassDiagram
class CMSAjaxBase {
+ajax_post(request, instance, parameter)
+get_form_class()
+get_form_fields()
}
class FormPlugin {
+form_spacing
+form_login_required
+form_unique
+form_actions
+get_form_fields()
}
FormPlugin --|> CMSAjaxBase
note for FormPlugin "Changed redirect behavior
from placeholder.page to
SAME_PAGE_REDIRECT"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment explaining why SAME_PAGE_REDIRECT is needed instead of placeholder.page, particularly in relation to form submissions from alias pages.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Summary by Sourcery
Fix form redirection issue when submitted from an alias and update framework support to include newer versions of Django and Django CMS. Update CI configuration to test against these newer versions.
Bug Fixes:
Enhancements:
CI: