8000 Fix date for dynamic contents by kuzmany · Pull Request #9955 · mautic/mautic · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix date for dynamic contents #9955

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jun 9, 2021
Merged

Fix date for dynamic contents #9955

merged 9 commits into from
Jun 9, 2021

Conversation

kuzmany
Copy link
Member
@kuzmany kuzmany commented May 2, 2021 •
edited by npracht
Loading
Q A
Branch? feature for 4.0
Bug fix? yes
New feature? no
Deprecations? no
BC breaks? no
Automated tests included? no
Related user documentation PR URL
Related developer documentation PR URL
Issue(s) addressed

Description:

Notice issue with date field in dynamics content

Steps to test this PR:

  1. Load up this PR
  2. Create custom contact date field
  3. Create dynamic content for landing page with filter on date field with not empty operator.
  4. Test dynamic content
  5. Without PR you always see variant for not empty, even if your field is empty.
  6. After PR you should see content properly

@kuzmany kuzmany added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels May 2, 2021
@kuzmany kuzmany added this to the 4.0-rc milestone May 2, 2021
@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label May 2, 2021
@codecov
Copy link
codecov bot commented May 2, 2021 •
edited
Loading

Codecov Report

Merging #9955 (7b9f73d) into features (9e5fcc8) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             features    #9955   +/-   ##
===========================================
  Coverage       41.53%   41.54%           
+ Complexity      34578    34575    -3     
===========================================
  Files            2063     2063           
  Lines          111566   111560    -6     
===========================================
+ Hits            46340    46347    +7     
+ Misses          65226    65213   -13
Impacted Files Coverage Δ
...ilBundle/EventListener/MatchFilterForLeadTrait.php 52.50% <ø> (+8.05%) ⬆️

npracht
npracht previously approved these changes May 6, 2021
View reviewed changes
Copy link
Member
@npracht npracht left a comment

Choose a reason for hiding this comment

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

Capture d’écran 2021-05-06 à 09 14 20

Works like a charm

@npracht npracht added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels May 6, 2021
@RCheesley
Copy link
Member

@kuzmany looks like we have a conflict to address here, could you take a look?

@RCheesley RCheesley added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label May 15, 2021
kuzmany added 2 commits May 15, 2021 18:34
@kuzmany
Merge remote-tracking branch 'upstream/features' into fix-date-dynami…
311940d 
…c-content

# Conflicts:
#	app/bundles/EmailBundle/Tests/EventListener/MatchFilterForLeadTraitTest.php
@kuzmany
Resolve conflicts in unit tests
1b165e2
@kuzmany
Copy link
Member Author
kuzmany commented May 15, 2021

@RCheesley conflicts resolved

RCheesley
RCheesley approved these changes May 15, 2021
View reviewed changes
Copy link
Member
@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this issue @kuzmany - I was able to reproduce the issue and when the PR was applied, the correct variant was shown based on the date field as described.

We will need a code review on this as neither of the two testers have reviewed the code, but then I think we are good to go!

@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 has-conflicts Pull requests that cannot be merged until conflicts have been resolved pending-test-confirmation PR's that require one test before they can be merged labels May 15, 2021
@RCheesley RCheesley requested a review from a team May 16, 2021 16:38
@npracht npracht merged commit 02c12cf into mautic:features Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RCheesley RCheesley RCheesley approved these changes

@npracht npracht npracht left review comments

Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement code-review-needed PR's that require a code review before merging dynamic-content ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Milestone
4.0.0-rc
Development

Successfully merging this pull request may close these issues.
3 participants
@kuzmany @RCheesley @npracht
0