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: account for unauthenticated readers when matching user_account criteria #1358

Closed
wants to merge 1 commit into from

Conversation

chickenn00dle
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

Closes https://app.asana.com/0/1208604368072430/1208670219439751/f

This PR addresses an issue where our criteria matching logic was not accounting for RAS readers that had logged out. We address this by also checking the RAS authenticated flag.

How to test the changes in this Pull Request:

  1. Set up two custom campaign prompts, one for logged in users and one for logged out users
  2. Add the prompt to any page or post
  3. As a reader, login and view the page or post where the prompt is set
  4. Verify the logged in prompt is visible
  5. Go to my account and log out
  6. Once again visit the prompt page
  7. On trunk the logged in prompt will still be present, on this branch the logged out prompt will be present.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@leogermani
Copy link
Contributor

It works as described in testing instructions but I'll defer the review to @miguelpeixe and @dkoo .

Technically, the segment is not for "logged out" readers, is for "readers that don't have an account"... So if they just logged but we know they have an account, should we display that prompt?

@chickenn00dle
Copy link
Contributor Author

Technically, the segment is not for "logged out" readers, is for "readers that don't have an account"... So if they just logged but we know they have an account, should we display that prompt?

Just FYI @leogermani, there is discussion in the Asana task about this: https://app.asana.com/0/1208604368072430/1208670219439751/f

@chickenn00dle
Copy link
Contributor Author

Closing per the comment here: 0-as-1208670219439751/1208837361414561/f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants