-
Notifications
You must be signed in to change notification settings - Fork 51
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(ras): skip campaign setup #3051
Conversation
added new route, added fe logic
added ui to indicate async skipping flow
combined "skipped" and "active" into single `isValid` const
added skip logic to RAS wizard
added allow continue when skipped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredrethman Thanks for diving into this without much context! The UI work is pretty much exactly what's needed aside from a minor suggestion below. However, we're missing two other requirements to truly skip the campaign setup:
- If skipping the Campaign step, we should not execute this call to
Reader_Activation::activate()
. This method is what actually generates the campaign prompts + segments, so to truly skip we shouldn't generate those at all. I suppose we could just set$response
to true in this case. - A more minor detail: if skipping, we should probably remove this step from the progress animation that shows when you click "activate" on the last step. If skipping campaign setup, we shouldn't be creating segments, so showing this step doesn't make sense.
The end result after skipping is that all prerequisites in the Reader Activation screen should be complete (as you've implemented already), but when visiting Newspack > Campaigns there shouldn't be anything here. Sorry if the business requirement wasn't clear—this feature is meant to support publishers who don't want to use the defaults at all.
return; | ||
} | ||
setSkipped( { isSkipped: Boolean( request.skipped ), status: '' } ); | ||
newspack_engagement_wizard.is_skipped_campaign_setup = request.skipped ? '1' : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this could we automatically transition to the next view? I clicked the "Skip" button and didn't realize I still had to click "Continue" to proceed.
@dkoo appreciate the review and feedback, but especially sharing insight into desired outcome. Some responses:
newspack-plugin/includes/wizards/class-engagement-wizard.php Lines 285 to 298 in 3110cb5
newspack-plugin/assets/wizards/engagement/views/reader-activation/complete.js Lines 68 to 70 in 3110cb5
I've tested with latest and no Segments + Campaigns were created.
Done 👍 Let me know if I missed anything and thanks again 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredrethman this is working really well now! Sorry about this, but I forgot about one other thing that doesn't make sense if you're skipping the campaign setup. In the /complete
view there's a list of "what happens next" before you click the "Enable Reader Activation" button:
![Screenshot 2024-04-15 at 1 10 16 PM](https://private-user-images.githubusercontent.com/2230142/322600189-c2211b34-5592-4ba8-bd55-4ca71e717745.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkxMDI5ODgsIm5iZiI6MTczOTEwMjY4OCwicGF0aCI6Ii8yMjMwMTQyLzMyMjYwMDE4OS1jMjIxMWIzNC01NTkyLTRiYTgtYmQ1NS00Y2E3MWU3MTc3NDUucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDlUMTIwNDQ4WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NGE5M2QwYzhmYzk0OGMxYmRkYzgwZmFhMWIyN2NmNmZmMWNjMDFlZjU4ZGIyZDYwZWVlMTRhNDNlYjgzNjNjMiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.nw5X4zn11v03KQioDbN-43-GmsoD7fNT5a6Pvm1x7YU)
If you've skipped the campaign setup, could we add [skipped] to the steps that won't happen? Something like this:
![Screenshot 2024-04-15 at 1 14 22 PM](https://private-user-images.githubusercontent.com/2230142/322600746-6fbd6ccd-2579-4c0f-af4c-dbead4013504.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkxMDI5ODgsIm5iZiI6MTczOTEwMjY4OCwicGF0aCI6Ii8yMjMwMTQyLzMyMjYwMDc0Ni02ZmJkNmNjZC0yNTc5LTRjMGYtYWY0Yy1kYmVhZDQwMTM1MDQucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDlUMTIwNDQ4WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YjlhMWViMTkxNjBlYzYwM2VlZDEzZGU3MmRkOGYzZGRlYzc3NzRhMjcxY2JlYjhkNzdjNjM5NWMwYTcwMGJlNiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.N3G_iGwBJHhuRzWgx1OxPE90XSSiUqNf-90f_JaswHQ)
Have updated with your feedback. Should now see the below, after your pull latest code with commit:feat: pr feedback, appended [skipped] to select items Let me know if there's anything else, thanks for such a thorough review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes, @jaredrethman!
@dkoo just double checking the status here. Can this be merged (into |
Yes, it can be merged! Sorry I wasn't clear. Approval = good to merge |
And when merging, please squash |
@jaredrethman – also when squashing, ensure the commit message follows the |
Went ahead and merged so it makes it into the next release cycle! |
# [4.0.0-alpha.1](v3.8.0...v4.0.0-alpha.1) (2024-04-25) ### Bug Fixes * ensure only admins can reset starter content and newspack options ([#3081](#3081)) ([4606721](4606721)) ### Features * add product option to autocomplete orders ([#3072](#3072)) ([4a2859b](4a2859b)) * **ras:** skip campaign setup ([#3051](#3051)) ([9ef0e6d](9ef0e6d)) ### BREAKING CHANGES * Changes order autocompletion behavior for existing products! * feat: add product option to autocomplete orders * chore: update outdated docblock description
🎉 This PR is included in version 4.0.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
* chore(deps-dev): bump @wordpress/browserslist-config Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 5.37.0 to 5.38.0. - [Release notes](https://github.com/WordPress/gutenberg/releases) - [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md) - [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/[email protected]/packages/browserslist-config) --- updated-dependencies: - dependency-name: "@wordpress/browserslist-config" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * chore(deps-dev): bump @rushstack/eslint-patch from 1.10.1 to 1.10.2 Bumps [@rushstack/eslint-patch](https://github.com/microsoft/rushstack/tree/HEAD/eslint/eslint-patch) from 1.10.1 to 1.10.2. - [Changelog](https://github.com/microsoft/rushstack/blob/main/eslint/eslint-patch/CHANGELOG.md) - [Commits](https://github.com/microsoft/rushstack/commits/@rushstack/eslint-patch_v1.10.2/eslint/eslint-patch) --- updated-dependencies: - dependency-name: "@rushstack/eslint-patch" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> * chore(deps-dev): bump @types/wordpress__blocks from 12.5.13 to 12.5.14 Bumps [@types/wordpress__blocks](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/wordpress__blocks) from 12.5.13 to 12.5.14. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/wordpress__blocks) --- updated-dependencies: - dependency-name: "@types/wordpress__blocks" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> * refactor(data-events): add abstract Connector class for shared methods (#3015) * refactor: add abstract Connector class for shared methods * chore: remove unused imports * chore(deps-dev): bump @wordpress/browserslist-config Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 5.38.0 to 5.39.0. - [Release notes](https://github.com/WordPress/gutenberg/releases) - [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md) - [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/[email protected]/packages/browserslist-config) --- updated-dependencies: - dependency-name: "@wordpress/browserslist-config" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * chore(deps): bump qs and @types/qs Bumps [qs](https://github.com/ljharb/qs) and [@types/qs](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/qs). These dependencies needed to be updated together. Updates `qs` from 6.12.0 to 6.12.1 - [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md) - [Commits](ljharb/qs@v6.12.0...v6.12.1) Updates `@types/qs` from 6.9.14 to 6.9.15 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/qs) --- updated-dependencies: - dependency-name: qs dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: "@types/qs" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> * fix: ensure only admins can reset starter content and newspack options (#3081) * feat: add product option to autocomplete orders (#3072) BREAKING CHANGE: Changes order autocompletion behavior for existing products! * feat: add product option to autocomplete orders * chore: update outdated docblock description * feat(ras): skip campaign setup (#3051) * feat(ras): skip campaign setup boilerplate added new route, added fe logic * feat(ras): added `is_skipped` to prerequisite status endpoint * refactor: moved callback to named function added ui to indicate async skipping flow * feat: added skip logic to prerequisite component combined "skipped" and "active" into single `isValid` const * feat: `is_skipped` property to prerequisite type added skip logic to RAS wizard * fix: circle ci / typescript * fix: ci / eslint * refactor: remove redirect after skip added allow continue when skipped. * refactor: allow parametrized skip * refactor: revert any type cast * feat: pr feedback - added redirect, activation skip and remove `activationStep[0]` * feat: pr feedback, appended `[skipped]` to select items * chore(release): 4.0.0-alpha.1 [skip ci] # [4.0.0-alpha.1](v3.8.0...v4.0.0-alpha.1) (2024-04-25) ### Bug Fixes * ensure only admins can reset starter content and newspack options ([#3081](#3081)) ([4606721](4606721)) ### Features * add product option to autocomplete orders ([#3072](#3072)) ([4a2859b](4a2859b)) * **ras:** skip campaign setup ([#3051](#3051)) ([9ef0e6d](9ef0e6d)) ### BREAKING CHANGES * Changes order autocompletion behavior for existing products! * feat: add product option to autocomplete orders * chore: update outdated docblock description * chore(release): 4.0.0-alpha.2 [skip ci] # [4.0.0-alpha.2](v4.0.0-alpha.1...v4.0.0-alpha.2) (2024-04-25) ### Bug Fixes * remove deprecated filter callback ([#3090](#3090)) ([5d7d0bf](5d7d0bf)) * chore(release): 4.0.0-alpha.3 [skip ci] # [4.0.0-alpha.3](v4.0.0-alpha.2...v4.0.0-alpha.3) (2024-04-26) ### Bug Fixes * enable Memberships fix cron job only when environment constant is defined ([#3087](#3087)) ([5d40297](5d40297)) * OAuth: use a custom table for transients (#3106) * feat: start new custom table for oauth transients data * fix: copy/paste error * fix(google-oauth): use custom table for transients * feat: cleanup old transients * fix: clean up on the fly too, and limit to deleting 1000 at a time max --------- Co-authored-by: Adam Cassis <[email protected]> * fix: autocomplete orders only for virtual products (#3111) * fix: autocomplete orders only for virtual products * chore: remove console.log --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: matticbot <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Derrick Koo <[email protected]> Co-authored-by: Jared Rethman <[email protected]> Co-authored-by: matticbot <[email protected]> Co-authored-by: dkoo <[email protected]>
🎉 This PR is included in version 4.0.0-epic-ras-acc.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [4.0.0](v3.8.8...v4.0.0) (2024-05-28) ### Bug Fixes * autocomplete orders only for virtual products ([#3111](#3111)) ([bfbe554](bfbe554)) * **data-events:** handle no ga_client_id in data ([fc4fc9f](fc4fc9f)) * ensure only admins can reset starter content and newspack options ([#3081](#3081)) ([4606721](4606721)) * **registration-block:** prevent undefined variable warning ([8af9d89](8af9d89)) * **starter-content:** make the starter content generation idempotent ([d5e10ff](d5e10ff)) ### Features * add product option to autocomplete orders ([#3072](#3072)) ([4a2859b](4a2859b)) * **cli:** command to migrate CAP guest authors to WP users ([#3068](#3068)) ([7ea8273](7ea8273)) * custom role for assignable authors who don't edit posts ([#3066](#3066)) ([7b89053](7b89053)) * **ras:** skip campaign setup ([#3051](#3051)) ([9ef0e6d](9ef0e6d)) * support for wrapping rss titles in cdata ([#3104](#3104)) ([8b3a3be](8b3a3be)) ### BREAKING CHANGES * Changes order autocompletion behavior for existing products! * feat: add product option to autocomplete orders * chore: update outdated docblock description
🎉 This PR is included in version 4.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Ability to allow editors to skip "Reader Activation Campaign" setup.
How to test the changes in this Pull Request:
git checkout origin/feat/skip-ras-campaign-setup && npm run build
Other information: