8000 Fix entity lookup type by kuzmany · Pull Request #9653 · mautic/mautic · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix entity lookup type #9653

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 7 commits into from
Apr 27, 2021
Merged

Fix entity lookup type #9653

merged 7 commits into from
Apr 27, 2021

Conversation

kuzmany
Copy link
Member
@kuzmany kuzmany commented Feb 8, 2021 •
edited by npracht
Loading
Q A
Branch? 3.3
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:

We noticed issue with lookup type If use two different lookup type from same model. This cause issue in segment email translations

Steps to test this PR:

  1. Load up this PR
  2. Create segment email (must be segment)
  3. Create second segment email as translation and set Is a translation of email create in step
  4. After save see Is a translation of is empty
    image
  5. With this PR works properly

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

Codecov Report

Merging #9653 (23f593e) into features (d236523) will increase coverage by 0.39%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##             features    #9653      +/-   ##
==============================================
+ Coverage       39.51%   39.91%   +0.39%     
  Complexity      34333    34333              
==============================================
  Files            2038     2038              
  Lines          108515   108602      +87     
==============================================
+ Hits            42884    43345     +461     
+ Misses          65631    65257     -374
Impacted Files Coverage Δ Complexity Δ
.../bundles/CoreBundle/Form/Type/EntityLookupType.php 100.00% <100.00%> (+2.17%) 8.00 <0.00> (-1.00) ⬆️
app/bundles/CoreBundle/Entity/CommonRepository.php 63.69% <0.00%> (+0.13%) 322.00% <0.00%> (ø%)
app/bundles/EmailBundle/Model/EmailModel.php 34.68% <0.00%> (+1.80%) 319.00% <0.00%> (ø%)
app/bundles/EmailBundle/Entity/EmailRepository.php 57.24% <0.00%> (+2.97%) 73.00% <0.00%> (ø%)
app/bundles/EmailBundle/Form/Type/EmailType.php 95.69% <0.00%> (+4.78%) 19.00% <0.00%> (ø%)
...bundles/EmailBundle/Controller/EmailController.php 27.69% <0.00%> (+5.59%) 145.00% <0.00%> (ø%)
...dles/CoreBundle/Templating/Helper/AssetsHelper.php 68.34% <0.00%> (+6.83%) 115.00% <0.00%> (ø%)
...pp/bundles/EmailBundle/Form/Type/EmailListType.php 100.00% <0.00%> (+6.89%) 3.00% <0.00%> (ø%)
...Form/Type/DynamicContentFilterEntryFiltersType.php 100.00% <0.00%> (+7.14%) 4.00% <0.00%> (ø%)
... and 25 more

@RCheesley RCheesley added the needs-automated-tests PR's that need automated tests before they can be merged label Feb 10, 2021
@RCheesley RCheesley modified the milestones: 3.3.1, 3.3.2 Feb 23, 2021
@npracht npracht added the email Anything related to email label Mar 5, 2021
RCheesley
RCheesley previously approved these changes Mar 13, 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.

Tested before applying and was unable to have the translation stay selected.

After applying the PR I was able to save and it retained the translated setting, and testing a send can confirm that the translated version was correctly sent to contacts who had that set in their contact profile.

Thanks @kuzmany ! 🚀

@RCheesley RCheesley added pending-test-confirmation PR's that require one test before they can be merged T1 Low difficulty to fix (issue) or test (PR) and removed ready-to-test PR's that are ready to test labels Mar 13, 2021
@RCheesley RCheesley modified the milestones: 3.3.2, 3.3.3 Mar 15, 2021
@ts-navghane ts-navghane requested a review from RCheesley April 13, 2021 07:11
@ts-navghane ts-navghane removed the needs-automated-tests PR's that need automated tests before they can be merged label Apr 13, 2021
@ts-navghane
Copy link
Contributor

I have tested this PR and works as expected, ready-to-commit label can be added.
There was a test case missing, which I have added.
Requesting @RCheesley for a review.

@RCheesley
Copy link
Member

Bumping this to the 4.0 beta release as it is based on the Features branch. Thanks for adding the test @ts-navghane !

@RCheesley RCheesley modified the milestones: 3.3.3, 4.0-beta Apr 13, 2021
@RCheesley
Copy link
Member

Updating the branch to re-run tests then we should be GTG 🚀

@RCheesley RCheesley removed the pending-test-confirmation PR's that require one test before they can be merged label Apr 26, 2021
@RCheesley RCheesley added the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Apr 26, 2021
@RCheesley
Copy link
Member

Uh oh ... seems like we have a failing test @kuzmany @ts-navghane https://github.com/mautic/mautic/pull/9653/checks?check_run_id=2438422003#step:9:69

@kuzmany
Copy link
Member Author
kuzmany commented Apr 26, 2021

@RCheesley fixed

@RCheesley RCheesley merged commit 9570a03 into mautic:features Apr 27, 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

Assignees
No one assigned
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 T1 Low difficulty to fix (issue) or test (PR)
Projects
None yet
Milestone
4.0.0-beta
Development

Successfully merging this pull request may close these issues.
6 participants
@kuzmany @ts-navghane @RCheesley @npracht @mohit-rocks @dennisameling
0