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

feat(nimbus): Add option to add targeting criteria on the Audience page #12384

Merged
merged 7 commits into from
Mar 27, 2025

Conversation

yashikakhurana
Copy link
Contributor

Because

  • We moved all the links like help, docs, reports etc on the top of the home page, but the targeting criteria is missing

This commit

  • Adds the option for advance targeting on the Audience page
  • Auto fill the details on github issue
  • Links to the doc

Fixes #12096
Screenshot 2025-03-26 at 7 15 25 PM

Screen.Recording.2025-03-26.at.7.15.04.PM.mov

Copy link
Contributor

@mikewilli mikewilli left a comment

Choose a reason for hiding this comment

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

Overall looks good, main issue is that I think the request URL should point to experimenter?

Also, tests?

@yashikakhurana
Copy link
Contributor Author

Overall looks good, main issue is that I think the request URL should point to experimenter?

Also, tests?

Thank you @mikewilli for the review, updated the URL, also for the tests we are not testing each page UI for htmx, but this will cover as part of the integration tests

Copy link
Contributor

@mikewilli mikewilli left a comment

Choose a reason for hiding this comment

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

Couple comments, but nothing too major so won't block on them.

TARGETING_CRITERIA_REQUEST = (
"https://github.com/mozilla/experimenter/issues/new"
"?template=targeting_request_template.yml"
"&title=Targeting%20criteria%20request"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this title override the default title you put in the template? If so, I'd leave it out of the URL (and update the template's title instead if needed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to do this testing once this gets landed, I just followed the instructions :)

name: Targeting Criteria Request
description: Request new advanced targeting logic for an experiment.
title: "[Targeting] "
labels: [targeting, enhancement]
Copy link
Contributor

Choose a reason for hiding this comment

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

Just confirming that we have both of these labels (if not, want to create them?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this

@yashikakhurana yashikakhurana added this pull request to the merge queue Mar 27, 2025
Merged via the queue into mozilla:main with commit bee26fb Mar 27, 2025
15 checks passed
@yashikakhurana yashikakhurana deleted the 12096/targeting_criteria branch March 27, 2025 15:04
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.

Add link to file targeting criteria on the summary page
2 participants