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: Checkbox group in form return bool #5366

Merged
merged 4 commits into from
Dec 20, 2017

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Nov 22, 2017

Q A
Bug fix? y
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs) #5348
BC breaks?
Deprecations?

Description:

This bugfix/feature has been sponsored by @Webmecanik

Form checkboxgroup return array then it wansn't possible match value to contact field and was ignored.
This fix just check if value is array and with 1 size and then convert array to value.

Steps to reproduce the bug:

  1. Create a boolean contact custom field with YES and NO (
  2. Create a form
  3. In this form add a checkox group field and map it to the previous custom field created.
  4. In properties tab, add a value (in the value, I fill the value that equals to Yes) I tried Yes, 1 and True.
  5. Fill the form and clic on the box.
  6. Go to contact sheet and check the value for the the custom field create before. There is no value. It should be Yes

Steps to test this PR:

  1. Same like steps to reproduce
  2. But see contact profile and his contact field bool value

image

@npracht
Copy link
Member

npracht commented Nov 23, 2017

Just tested it works ! 👍

@johbuch
Copy link
Contributor

johbuch commented Nov 23, 2017

@kuzmany
I tested also.
I got this field (mapped to a boolean field)
image
If I answer Yes > I got the good value which is 1
image
But if I choose No > I got nothing on the contact sheet
image
On the form field, I tested with 1 and 0 as values
image
But also with true and false. It is always the same

@kuzmany
Copy link
Member Author

kuzmany commented Nov 23, 2017

@johbuch works just with one option in checkbox group.
If you are using more checkbox options then you need use the same or similar custom contact field with multiple options support.

This patch resolve just one option and covert value from array to normal value. Naturally

@npracht
Copy link
Member

npracht commented Nov 24, 2017

So if i understand well, for this 2 options boolean, it shouldn't be a checkbox group but a radio button right ?

Thanks !

@kuzmany
Copy link
Member Author

kuzmany commented Nov 24, 2017

@npracht yeas multiple checkboxes return multiple values, then we can't save multi values (array) to single contact field.
For multiple options use radio buttons with one result use radio button.
For one option use checkbox.

@floriannicolas
Copy link

Hi, I have the same problem, but my custom field isn't a boolean : it is a Select data type with only two values : "Yes" and "No" (don't ask me why I didn't use a boolean, I don't know).

I previously used a checkbox form field to update my custom field in a form, but since last release (2.11.0), my value isn't saved anymore. (I'm now using two radio inputs while waiting for a solution.)

I thought this patch would solve my problem, but sadly no.
But it's weird because by reading the patch I really thought he was going to solve it!

@alanhartless alanhartless added this to the 2.12.1 milestone Dec 13, 2017
@alanhartless alanhartless added the bug Issues or PR's relating to bugs label Dec 13, 2017
@alanhartless alanhartless added the pending-test-confirmation PR's that require one test before they can be merged label Dec 19, 2017
@javjim javjim self-assigned this Dec 19, 2017
@javjim
Copy link

javjim commented Dec 19, 2017

Was able to reproduce bug and fix. Thanks

@javjim javjim added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Dec 19, 2017
@javjim javjim removed their assignment Dec 19, 2017
@alanhartless alanhartless merged commit 73cbb1c into mautic:staging Dec 20, 2017
@dbhurley dbhurley removed the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Dec 20, 2017
@kuzmany kuzmany deleted the chekbox-group-bool-fix branch April 7, 2018 17:55
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants