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

Set roundmode on attribution field #1956 #1957 #1963

Merged
merged 2 commits into from Jul 8, 2016

Conversation

wittwerch
Copy link
Contributor

Please answer the following questions:

Q A
Bug fix? y
New feature? n
BC breaks? n
Deprecations? n
Fixed issues #1956 #1957

Description

The installer creates the lead fiels using Doctrine fixtures. The fixtures are missing the round mode for the new field attribution, which breaks the contact form.

Steps to reproduce the bug (if applicable)

Steps to test this PR

@escopecz
Copy link
Sponsor Member

escopecz commented Jul 6, 2016

Looks good. Are there any steps to replicate? I can edit contacts without any issue.

@wittwerch
Copy link
Contributor Author

Yes, it only happens when you install Mautic 2.0 with the installer. Upgrading works, because the doctrine migration sets this value:

(1,'Attribution', 'attribution', 'number', 'core', '0', 0, 1, 1, 0, 1, 0, 0, 23, 'a:2:{s:9:\"roundmode\";s:1:\"4\";s:9:\"precision\";s:1:\"2\";}'),

@alanhartless alanhartless modified the milestone: 2.0.1 Jul 6, 2016
@mqueme
Copy link
Contributor

mqueme commented Jul 7, 2016

I am unable to replicate. Can you explain how the form gets broken please.

@wittwerch
Copy link
Contributor Author

  1. Checkout release 2.0 from Github
  2. Bootstrap using the installer
  3. Go to Contacts -> New
  4. Fill in firstname, lastname and e-mail
  5. Click on "Save & Close"

Wheel keeps spinning forever. You get a HTTP 200 from the backend, but it contains the error message about the roundmode missing.

@escopecz
Copy link
Sponsor Member

escopecz commented Jul 7, 2016

I was able to replicate with the new installation like you said. The thing is that your fix will work for the new installations, but won't fix already installed Mautics 2.0.0. Would you mind to replace

https://github.com/mautic/mautic/blob/staging/app/bundles/LeadBundle/Form/Type/LeadType.php#L148

with

'rounding_mode' => isset($properties['roundmode']) ? (int) $properties['roundmode'] : 0

and add it to your PR?

@escopecz
Copy link
Sponsor Member

escopecz commented Jul 8, 2016

Thanks! As I'm looking at the code, we should probably unify the roundmode to 0 or 2. Not sure which should be the default. But that's just a minor issue. This PR is good to go by me 👍

@escopecz escopecz added T3 Hard difficulty to fix (issue) or test (PR) bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test pending-test-confirmation PR's that require one test before they can be merged labels Jul 8, 2016
@escopecz escopecz merged commit a58cc78 into mautic:staging Jul 8, 2016
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 pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test T3 Hard difficulty to fix (issue) or test (PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants