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

Between operator for segments with numeric Custom Object fields #346

Merged
merged 12 commits into from
Jun 14, 2024

Conversation

aarohiprasad
Copy link
Contributor

@aarohiprasad aarohiprasad commented May 16, 2024

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

Description:

This PR implements the between operator in segments for numeric fields.

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Create a Custom Object People/Person and add a number field "Family Members".
  3. Create 3 custom items from this object; 'A', 'B' and 'C' and assign them 3, 7 and 12 family members respectively.
  4. Create 3 contacts 'A', 'B' and 'C' and link them to People 'A', 'B' and 'C' respectively.
  5. Create a new segment "Family Members Between Segment" and add People:Family Members as filter field.
  6. Choose "between" operator. Two input fields should appear.
image
  1. Enter 5 and 10 in the "From" and "To" boxes respectively and save the segment.
  2. Wait for the segment to build.
  3. Verify that only contact B (the one with 7 family members) is a member of the segment.
  4. Additionally try saving the segment leaving either of the two input fields blank, or entering non integer values. You should see an error message.
image 11. Create another segment, "Family Members Not Between Segment", with same configurations as the previous one. Except this time choose "not between" operator.
  1. Wait for the segment to build and verify that this time the contacts with 3 and 12 family members are members of the segment.

Other areas of Mautic that may be affected by the change: NA

@aarohiprasad aarohiprasad requested review from a team, dadarya0 and ts-navghane and removed request for a team May 16, 2024 13:29
@avikarshasaha avikarshasaha requested review from avikarshasaha and removed request for avikarshasaha May 16, 2024 14:35
@avikarshasaha avikarshasaha requested review from avikarshasaha and removed request for avikarshasaha May 20, 2024 06:46
@fedys fedys requested review from avikarshasaha and escopecz and removed request for dadarya0 and ts-navghane May 20, 2024 09:48
Helper/QueryFilterHelper.php Outdated Show resolved Hide resolved
Copy link
Contributor

@avikarshasaha avikarshasaha left a comment

Choose a reason for hiding this comment

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

@aarohiprasad Please re-request for review when it is done.

Helper/QueryFilterHelper.php Outdated Show resolved Hide resolved
escopecz
escopecz previously approved these changes May 21, 2024
Copy link
Contributor

@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.

I see no more issues, thanks!

@aarohiprasad
Copy link
Contributor Author

Only one commit has been added since the last review. Please review 8de83d7. @escopecz @avikarshasaha. Thank you!

escopecz
escopecz previously approved these changes May 23, 2024
Copy link
Contributor

@avikarshasaha avikarshasaha left a comment

Choose a reason for hiding this comment

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

This is not working in the beta version. @aarohiprasad is having some issues with the merging. She will fix it.

avikarshasaha
avikarshasaha previously approved these changes May 27, 2024
@fedys fedys dismissed stale reviews from avikarshasaha and escopecz via 308fb0f May 30, 2024 15:06
@aarohiprasad aarohiprasad force-pushed the MAUT-11287 branch 2 times, most recently from 5f1636c to fc96692 Compare June 4, 2024 11:47
escopecz
escopecz previously approved these changes Jun 6, 2024
Copy link
Contributor

@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.

reapproving

avikarshasaha
avikarshasaha previously approved these changes Jun 7, 2024
@avikarshasaha avikarshasaha dismissed stale reviews from escopecz and themself via 204c3c1 June 14, 2024 08:20
Copy link
Contributor

@rahuld-dev rahuld-dev left a comment

Choose a reason for hiding this comment

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

approving PR after resolved conflicts

@avikarshasaha avikarshasaha merged commit d351b6f into staging Jun 14, 2024
3 checks passed
@avikarshasaha avikarshasaha deleted the MAUT-11287 branch June 14, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants