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

Translate error message, simplify error handling #75

Merged
merged 3 commits into from May 24, 2023

Conversation

tschorr
Copy link
Contributor

@tschorr tschorr commented May 23, 2023

RegistrationTool.pasValidation expects translated error messages, however the password policy plugin doesn't translate the minimum length error message. Also simplify the error handling.

This is a follow up to plone/plone.restapi#1630 - the idea is to use the correctly rendered error message for display in Volto's PasswordReset.jsx (without this fix, the actual value of min_chars will not be rendered).

@mister-roboto
Copy link

@tschorr thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@tschorr
Copy link
Contributor Author

tschorr commented May 23, 2023

@jenkins-plone-org please run jobs

@tschorr tschorr self-assigned this May 23, 2023
@tschorr tschorr requested review from davisagli and jensens May 23, 2023 06:18
@tschorr tschorr changed the title WIP: Translate error message, simplify error handling Translate error message, simplify error handling May 23, 2023
@tschorr
Copy link
Contributor Author

tschorr commented May 23, 2023

Jenkins failures are actually meaningful:

Failure in test testPasValidation (Products.CMFPlone.tests.testRegistrationTool.TestRegistrationTool.testPasValidation)
Traceback (most recent call last):
  File "/srv/python3.11/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/srv/python3.11/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/srv/python3.11/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
  File "/home/jenkins/workspace/pull-request-6.1-3.11/src/Products.CMFPlone/Products/CMFPlone/tests/testRegistrationTool.py", line 150, in testPasValidation
    self.assertEqual(
  File "/srv/python3.11/lib/python3.11/unittest/case.py", line 873, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/srv/python3.11/lib/python3.11/unittest/case.py", line 1253, in assertMultiLineEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/srv/python3.11/lib/python3.11/unittest/case.py", line 703, in fail
    raise self.failureException(msg)
AssertionError: 'Your password must contain at least 8 characters.' != 'Your password must contain at least ${min_chars} characters.'
- Your password must contain at least 8 characters.
?                                     ^
+ Your password must contain at least ${min_chars} characters.
?                                     ^^^^^^^^^^^^

-> test is assuming the incorrect error message, we need to fix the test after merging this PR.

Copy link
Sponsor Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

@tschorr Thanks for working on this. I think there's one thing missing...

src/Products/PlonePAS/plugins/passwordpolicy.py Outdated Show resolved Hide resolved
@jensens
Copy link
Sponsor Member

jensens commented May 24, 2023

@jenkins-plone-org please run jobs

@tschorr tschorr merged commit 3aed660 into master May 24, 2023
4 of 7 checks passed
@tschorr tschorr deleted the tschorr_fix_error_msg branch May 24, 2023 10:35
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.

None yet

4 participants