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 a JS failure when the show pause preferences setting is disabled #11431

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

volha-pivavarchyk
Copy link
Contributor

Q A
Bug fix? (use the a.b branch) [ ]
New feature/enhancement? (use the a.x branch) [ ]
Deprecations? [ ]
BC breaks? (use the c.x branch) [ ]
Automated tests included? [ ]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

If the Show pause contact preferences date is not enabled the JS will fail.

This commit fixes this and does some general JS improves that will help with readablility and maintainability.

Steps to reproduce:

  1. Disable the "Show pause contact preferences" in Settings
  2. Send a segment email
  3. Click on the unsubscribe link
  4. Try to switch on/off the "I want to receive email" option
  5. A JS error will be shown in the console
    image
    The fields will not be enabled/disabled
    image

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. The same steps as in the reproduction part
  3. There is no the JS error. The fields can be enabled/disabled

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Sep 2, 2022
@escopecz
Copy link
Sponsor Member

escopecz commented Sep 2, 2022

I have also fixed this bug in our fork recently. It's slightly different solution. I'd like to avoid conflicts. I cannot push it directly as it depends on other PR that hasn't been pushed yet. I'll try to push it to unblock.

@adiux adiux requested a review from RCheesley September 5, 2022 12:41
@adiux adiux added the bug Issues or PR's relating to bugs label Sep 5, 2022
@escopecz
Copy link
Sponsor Member

escopecz commented Sep 7, 2022

@volha-pivavarchyk this is our version of the same bug fix. Please take a look: #11443

Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

Let's go with this change as #11443 has too many dependent PRs and will take some time to merge.

The code looks great 👍

@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged email Anything related to email labels Sep 12, 2022
@RCheesley RCheesley mentioned this pull request Sep 23, 2022
21 tasks
Copy link
Contributor

@jos0405 jos0405 left a comment

Choose a reason for hiding this comment

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

Works. There is no JS error after PR.

@RCheesley RCheesley added code-review-needed PR's that require a code review before merging ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed code-review-needed PR's that require a code review before merging pending-test-confirmation PR's that require one test before they can be merged labels Sep 23, 2022
@RCheesley RCheesley added this to the 4.4.3 milestone Sep 23, 2022
@RCheesley RCheesley merged commit c706767 into mautic:4.4 Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement email Anything related to email ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants