-
Notifications
You must be signed in to change notification settings - Fork 59
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
Sync upstream dec 13th #245
Sync upstream dec 13th #245
Conversation
Currently translated at 100.0% (1508 of 1508 strings) Co-authored-by: Tobias Kunze <[email protected]> Translate-URL: http://translate.pretalx.com/projects/pretalx/pretalx/de/ Translation: pretalx/pretalx
Currently translated at 100.0% (1508 of 1508 strings) Co-authored-by: Tobias Kunze <[email protected]> Translate-URL: http://translate.pretalx.com/projects/pretalx/pretalx/de_FORMAL/ Translation: pretalx/pretalx
closes #1864
closes #1909
closes #1908
Currently translated at 99.7% (1508 of 1512 strings) Co-authored-by: r spoor <[email protected]> Translate-URL: http://translate.pretalx.com/projects/pretalx/pretalx/nl/ Translation: pretalx/pretalx
…sasia#230) Co-authored-by: odkhang <[email protected]>
…sia#232) * Fix join online video button after customer feature is removed * fix pipeline, unit test --------- Co-authored-by: odkhang <[email protected]>
* change default color for eventyay-talk and some messages * change default color for eventyay-talk and some messages * fix UT --------- Co-authored-by: odkhang <[email protected]>
* Ensure SSO works on admin level and change implementation to be an external plugin * remove unsed template * fix pipeline * using provider name constant instead of text * update transaction for storing key * handle case sso_provider not configured --------- Co-authored-by: odkhang <[email protected]>
* Enable video plugin * Fix black in pipeline * Fix black in pipeline * Add comment * Update code --------- Co-authored-by: odkhang <[email protected]>
Reviewer's Guide by SourceryThis pull request synchronizes changes from upstream, focusing on significant architectural improvements around email templates, user management, and UI enhancements. The changes include a major refactor of the email template system, improved security features, and various bug fixes and optimizations. Class diagram for Email Template RefactorclassDiagram
class Event {
+get_mail_template(role)
+build_initial_data()
+reorder_review_phases()
+update_review_phase()
}
class MailTemplate {
+role: MailTemplateRoles
+to_mail(user, event, context_kwargs, skip_queue, commit, submissions, attachments)
}
class MailTemplateRoles {
<<enumeration>>
+NEW_SUBMISSION
+NEW_SUBMISSION_INTERNAL
+SUBMISSION_ACCEPT
+SUBMISSION_REJECT
+NEW_SPEAKER_INVITE
+EXISTING_SPEAKER_INVITE
+QUESTION_REMINDER
+NEW_SCHEDULE
}
Event --> MailTemplate : uses
MailTemplate --> MailTemplateRoles : has role
Class diagram for User Management ChangesclassDiagram
class User {
+change_password(new_password)
+reset_password(event, user, mail_text, orga)
}
class QueuedMail {
+send(requestor, orga)
}
User --> QueuedMail : sends
Class diagram for Submission ChangesclassDiagram
class Submission {
+add_speaker(email, name, locale, user)
+send_initial_mails(person)
+send_state_mail()
}
class SpeakerProfile {
}
Submission --> SpeakerProfile : manages
Submission --> User : adds speaker
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…/eventyay-talk into Sync-Upstream-Dec-13th
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 @HungNgien - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Setting CSRF_COOKIE_HTTPONLY to False exposes the CSRF token to XSS attacks (link)
- Removing field.value assignment breaks color value update functionality (link)
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 2 other issues
- 🔴 Security: 1 blocking issue
- 🟢 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.
|
||
Your pretalx account password was just changed. | ||
|
||
If you did not cange your password, please contact the site administration immediately. |
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.
issue (typo): Fix typo in password change email text
If you did not cange your password, please contact the site administration immediately. | |
If you did not change your password, please contact the site administration immediately. |
if total_length > max_length: | ||
# If the total length of the path exceeds the max length, we need to | ||
# shorten the file name by the difference. | ||
file_root = file_root[: -(total_length - max_length)] |
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.
issue: Add validation to ensure shortened filename is not empty
If the shortened filename would be empty, we should either raise an error or use a minimum length fallback.
@@ -160,6 +160,9 @@ def merge_csp(*options, config=None): | |||
|
|||
CSRF_COOKIE_NAME = "pretalx_csrftoken" | |||
CSRF_TRUSTED_ORIGINS = [SITE_URL] | |||
CSRF_COOKIE_SECURE = True | |||
CSRF_COOKIE_HTTPONLY = False |
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.
🚨 issue (security): Setting CSRF_COOKIE_HTTPONLY to False exposes the CSRF token to XSS attacks
The CSRF cookie should be httponly unless there is a specific requirement otherwise
@@ -43,7 +43,6 @@ const updateContrast = (field, color) => { | |||
".colorpicker-preview", | |||
).style.backgroundColor = color.hex | |||
// We're getting RRGGBBAA, but we don't want the alpha channel |
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.
issue (bug_risk): Removing field.value assignment breaks color value update functionality
The color value needs to be updated in the field, even if we're trimming the alpha channel
Summary by Sourcery
Sync with upstream changes, introducing a new mail template role system, improving email and submission handling, and updating dependencies. Enhance the CI setup by migrating to GitHub Actions and improve test reliability. Update translations and documentation for multiple languages.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests: