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

New campaign condition: Has active notification #6329

Merged

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Jul 16, 2018

Please be sure you are submitting this against the staging branch.

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

Description:

Just added new condition based on active notification from push_ids table.
Prevent too much failed campaign Send notification action.

Steps to test this PR:

  1. Setup OneSignal an web push Notification
  2. Create campaign with Has active notification.
  3. Go to website with tracking code and active notification.
  4. Go to website with incognito (another anonymouse contact was created)
  5. Then both contact add to campaign with Has active notification
  6. First should go to true, second to false

@npracht npracht added WIP PR's that are not ready for review and are currently in progress enhancement Any improvement to an existing feature or functionality labels Jul 16, 2018
@kuzmany kuzmany added ready-to-test PR's that are ready to test and removed WIP PR's that are not ready for review and are currently in progress labels Jul 17, 2018
@kuzmany kuzmany added this to the 2.14.1 milestone Jul 17, 2018
@YosuCadilla
Copy link

@kuzmany , I was tasked to test this but I am having a hard time understanding the steps to test this PR, please elaborate, thank you.

@kuzmany
Copy link
Member Author

kuzmany commented Aug 9, 2018

@YosuCadilla Just test two contacts.
First - notification activated
Second - no notifcations

Descriptions is for developers.

@YosuCadilla
Copy link

@kuzmany , Developers can code, marketers and other non coders can test.
I suggest that for the devs to be able to concentrate on their valuable skills, we use simpler to understand descriptions (for example the names used in the UI) so non coders can help and coders can code and not lose so much time in testing (whenever that's possible).

@kuzmany
Copy link
Member Author

kuzmany commented Aug 9, 2018

@YosuCadilla until 2.14.1 mostly developers tested. But this PR description is just an exception. Please test If contact has or hasn't notification (OneSignal). That's all
Description update

@heathdutton
Copy link
Member

I tested this and it appears to function correctly. https://mautibox.com/6329/s/campaigns/edit/2

@heathdutton heathdutton added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed ready-to-test PR's that are ready to test labels Sep 4, 2018
@heathdutton heathdutton merged commit 86bbcb6 into mautic:staging Sep 4, 2018
@escopecz escopecz removed ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged labels Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any improvement to an existing feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants