fix: fallback if no newspack plugin #1179

dkoo · 2023-08-08T21:34:33Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Segmentation and frequency-based features in the rearchitected plugin rely on APIs from the main Newspack Plugin. This PR ensures that if those APIs are not available, prompts will still be displayed, albeit without any regard to segmentation or frequency options.

Closes 1204189396255901/1204850211981984.

How to test the changes in this Pull Request:

Check out this branch and disable the main plugin: wp plugin deactivate newspack-plugin
In WP admin, confirm that the Prompts menu item is available and that it lists all prompts in a standard CPT UI.
Edit a prompt and confirm that the "Frequency" and "Segments" sidebar panels don't appear, as those options won't work without the main Newspack plugin.
View the front-end and confirm that all published prompts are displayed (although only one overlay prompt will be displayed even if multiple are published).
Reactivate the main Newspack plugin and confirm that all segmentation and frequency-related features work as expected again.

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?

…k Plugin

leogermani · 2023-08-09T14:03:54Z

includes/class-newspack-popups-inserter.php

+				$script_data['segments'] = self::$segments;
+
+			} else {
+				$script_data['segmentation_disabled'] = true;


nit: can we avoid the negative name here? It can become very confusing (as we saw yesterday)

Good call. d4b9a25 removes it, as there's really no need for a separate variable at all. More simply, if there are no segments to handle, then there's no need to use the reader data library at all.

Whoops, ef8ac67 fixes a bug in the prior commit which assumed that newspack_popups_view.segments would be an array and therefore have a length property, when in fact it's a keyed object.

…lugin

leogermani · 2023-08-11T19:09:08Z

hey @dkoo , I fixed the conflict after my PR was merged to the epic branch.

Also, now that Segments use the core taxonomy UI in the Editor, we needed a different approach to hide it.

It tests well for me but can you double-check it?

PS - I tried to not even register the taxonomy if segmentation is disabled but then many things that rely on it started to break, so I figured it would be simpler to just hide it

…lugin

dkoo

@leogermani Changes look good to me, but I can't currently test. If they're working for you with and without Newspack Plugin active, that's good enough for me. Feel free to approve or request another PR from @miguelpeixe or @laurelfulford for a second set of eyes.

* feat: segmentation criteria system (#1155) * chore: remove Lightweight API and client ID handling (#1166) * chore: remove Lightweight API and client ID handling * chore: remove data pruning cron job and clear future instances * fix: script dependencies * fix: remove unused method refs * test: remove tests using deprecated features * chore: remove more unneeded files * test: remove other failing tests * fix(criteria): warn when criteria value is not available (#1167) * feat: integrate criteria with segments (#1159) Co-authored-by: Leo Germani <[email protected]> * fix(criteria): allow async matching configuration (#1175) * feat: segmentation match logic (#1169) * feat: segmentation criteria system * chore: refactor default criteria and implement utils * chore: api tweaks * fix: range logic * chore: isolate segment matching example * fix: ras dependency placement * fix: add referrer sources category * fix: api tweaks for consistency * chore: file structure * chore: inline docs * chore: api tweaks * fix: schema tweaks * feat: add schema and migration helper * fix: rename newsletter subscriber store item * fix: send segment config instead of value * chore: move default criteria registration * fix: class name * fix: change criteria options values * fix: tweak migration * fix: remove referrer creation * fix: remove referrer creation * test: dont enqueue scripts on tests * test: add criteria * test: add required * test: add criteria * fix: logic typo * test: frontend matching * fix: range config * fix: missing range can be any range * test: php criteria registration * test: fix php test * test: fix typo * chore: improve inline docs * fix: update segment criteria schema * fix: configuration migration * feat: use criteria to match prompt segments * feat: fix schema and tests * refactor: file structure * chore: remove AMP polyfill JS * feat: implement scroll triggers and frequency * fix: add .phpunit.result.cache to .gitignore * refactor: use prompt and segment config via JSON instead of DOM * test: remove unit tests related to AMP Access and GA3 analytics * fix: only show one overlay per page * revert: use prompt and segment config via JSON instead of DOM * test: re-fix failing tests * fix: close overlay on background tap * fix: src should not be added to release packages * test: unit tests for segmetnation API * test: actual unit tests for segmentation API * chore: fix inline comment for accuracy * fix: race condition when using perfmatters * refactor: trigger seen GA4 event based on prompt_seen activity * refactor: make the 1x overlay at a time rule testable * refactor: overlay display check according to new unit test * test: update test assertion for accuracy * fix(criteria): allow async matching configuration * fix: ensure global * fix: logic for one-at-a-time overlays * Revert "fix: race condition when using perfmatters" This reverts commit c9fab16. * fix: remove defaultCriteria.js * fix: race condition with reader library JS --------- Co-authored-by: Miguel Peixe <[email protected]> Co-authored-by: Leo Germani <[email protected]> * feat: favorite categories segmentation logic (#1177) * feat: logic for favorite categories * fix: top-ranked category must actually have more views than the other categories * fix: simplify logic and account for undefined countsArray * fix: account for multiple views in one category; simplify logic a bit * fix: correct indexOf comparison * feat: migrate reader data (#1176) * fix: a PHP warning when migrating a user to the new data schema (#1180) * Refactor/segments relationship (#1168) * feat: refactor segment relationship * feat: refactor complete and tests updated * fix: popup exporter * fix: popup analytics events * fix: avoid warning in the schema This is actually a bug in core. see https://core.trac.wordpress.org/ticket/56494 * fix: do not return disabled segments on REST --------- Co-authored-by: Miguel Peixe <[email protected]> * feat: Update/remove category cleanup (#1185) * fix: remove old category cleanup It was looking at deprecated configuration meta After the refactor having a non existent cat should not be a problem * test: remove test * chore: remove lightweight api setup (#1183) * fix: fallback if no newspack plugin (#1179) * fix: allow prompts to be displayed without segmentation if no Newspack Plugin * refactor: avoid use of negative variable name * fix: newspack_popups_view.segments is not an array * fix: hide segments taxonomy --------- Co-authored-by: Leo Germani <[email protected]> Co-authored-by: dkoo <[email protected]> * fix: restore support for plugin settings (#1181) * fix: support segmentation on donor landing page * fix: support Mailchimp donor merge fields for segmentation --------- Co-authored-by: Leo Germani <[email protected]> * fix: lines added by accident in last merge * feat: move migration methods (#1184) Co-authored-by: Derrick Koo <[email protected]> * fix: auto-fill segment (#1188) * feat: remove non interactive mode (#1190) * fix: missing release files (#1191) * fix: distignore should include some src subfolders * fix: segments admin URL localized key name * fix: previews (#1178) * fix: allow manual prompts to be displayed in previews * fix: previewing as a segment * feat: use temporary reader data sessions for previews * fix: single-prompt previews * refactor: use helper function for determining override * test: add tests for view_as and pid overrides * test: fix query string in test --------- Co-authored-by: Miguel Peixe <[email protected]> Co-authored-by: Leo Germani <[email protected]> Co-authored-by: Miguel Peixe <[email protected]> Co-authored-by: dkoo <[email protected]>

fix: allow prompts to be displayed without segmentation if no Newspac…

ffbf39e

…k Plugin

dkoo added the [Status] Needs Review label Aug 8, 2023

dkoo requested a review from a team as a code owner August 8, 2023 21:34

dkoo self-assigned this Aug 8, 2023

leogermani reviewed Aug 9, 2023

View reviewed changes

refactor: avoid use of negative variable name

d4b9a25

dkoo requested a review from leogermani August 9, 2023 16:19

dkoo and others added 3 commits August 9, 2023 14:26

fix: newspack_popups_view.segments is not an array

ef8ac67

Merge branch 'epic/rearchitecture' into fix/fallback-if-no-newspack-p…

873a02b

…lugin

fix: hide segments taxonomy

b42006b

Merge branch 'epic/rearchitecture' into fix/fallback-if-no-newspack-p…

5b76d36

…lugin

dkoo commented Aug 14, 2023

View reviewed changes

leogermani merged commit 380765a into epic/rearchitecture Aug 15, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fallback if no newspack plugin #1179

fix: fallback if no newspack plugin #1179

dkoo commented Aug 8, 2023 •

edited

Loading

leogermani Aug 9, 2023

dkoo Aug 9, 2023 •

edited

Loading

dkoo Aug 9, 2023

leogermani commented Aug 11, 2023

dkoo left a comment •

edited

Loading

fix: fallback if no newspack plugin #1179

fix: fallback if no newspack plugin #1179

Conversation

dkoo commented Aug 8, 2023 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

leogermani Aug 9, 2023

Choose a reason for hiding this comment

dkoo Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

dkoo Aug 9, 2023

Choose a reason for hiding this comment

leogermani commented Aug 11, 2023

dkoo left a comment • edited Loading

Choose a reason for hiding this comment

dkoo commented Aug 8, 2023 •

edited

Loading

dkoo Aug 9, 2023 •

edited

Loading

dkoo left a comment •

edited

Loading