Skip to content

feat(corrections-location): integrate Correction location logic#3829

Merged
Takshil-Kunadia merged 12 commits into
trunkfrom
fix/corrections-location
Mar 18, 2025
Merged

feat(corrections-location): integrate Correction location logic#3829
Takshil-Kunadia merged 12 commits into
trunkfrom
fix/corrections-location

Conversation

@Takshil-Kunadia

Copy link
Copy Markdown
Collaborator

All Submissions:

Changes proposed in this Pull Request:

Closes https://app.asana.com/1/26890605006346/project/1209292256643614/task/1209635079338293.

How to test the changes in this Pull Request:

  1. Create a Test Post > Manage Corrections.
  2. Check Manage Corrections modal should have a new LOCATION setting. Default: Bottom.
  3. Add at some corrections with the location set to "top" and others with the location set to "bottom".
  4. Verify that each correction’s meta field for location is correctly saved. After reloading few times.
  5. View the post on the front-end and confirm that corrections with the "top" location are displayed above the post content and corrections with the "bottom" location are displayed below the post content.
  6. Ensure that if no corrections exist for a given location, no corrections in that location is rendered.

Next Steps:-

  • Refactor Corrections blocks to work with the location setting.

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?

@Takshil-Kunadia Takshil-Kunadia marked this pull request as ready for review March 13, 2025 08:41
@Takshil-Kunadia Takshil-Kunadia requested a review from a team as a code owner March 13, 2025 08:41
@Takshil-Kunadia Takshil-Kunadia added the [Status] Needs Review The issue or pull request needs to be reviewed label Mar 13, 2025

@leogermani leogermani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I did one small tweak to change "Close & save" to "Save & close"

@github-actions github-actions Bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Mar 13, 2025
@leogermani

Copy link
Copy Markdown
Contributor

One more thing. I just updated the docs -> 85768ab

Also, @Takshil-Kunadia , I was looking at the CORRECTIONS_ACTIVE_META. Do we still use it? I don't think there's a UI for that and, in the code, I only see us checking for it, but I don't see us writing this value... I think we can remove it? WDYT?

@leogermani leogermani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing approval while we discuss my last comment about the "is active" post meta

@Takshil-Kunadia

Copy link
Copy Markdown
Collaborator Author

@leogermani I too had this question before.

Basically this can be used as a flag for cases where Editor wants to keep the corrections saved(exist/persist) for that post but turn off displaying it in the front-end for a particular post as otherwise current logic will display corrections for the posts which have corrections data. If this is a use case...otherwise it makes sense to remove it.

@Takshil-Kunadia Takshil-Kunadia removed the [Status] Approved The pull request has been reviewed and is ready to merge label Mar 14, 2025
@thomasguillot

Copy link
Copy Markdown
Contributor

Design-wise, I haven't much to say 👍

@thomasguillot thomasguillot removed their request for review March 17, 2025 13:56
@leogermani

Copy link
Copy Markdown
Contributor

Basically this can be used as a flag for cases where Editor wants to keep the corrections saved(exist/persist) for that post but turn off displaying it in the front-end for a particular post as otherwise current logic will display corrections for the posts which have corrections data. If this is a use case...otherwise it makes sense to remove it.

We need to either remove it from the code or to add the UI to enable/disable it.

I think we can remove it, but then we need to change how we are handling the shortcode there.. because we are filtering by posts with this feature enabled. If we remove that we are going to quey all posts and we don't want that

@Takshil-Kunadia

Copy link
Copy Markdown
Collaborator Author

I think we can remove it, but then we need to change how we are handling the shortcode there.. because we are filtering by posts with this feature enabled. If we remove that we are going to quey all posts and we don't want that

@leogermani I don't think this should be an issue, right? we are already querying all the corrections of all posts with the get_posts(), filtering is done with the meta key but since we haven't provided UI I don't imagine it being set by anyone via a manual DB update.
moreover, eliminating meta query should be better for query instead.

@leogermani

Copy link
Copy Markdown
Contributor

I'm talking about this query ->

We need to change that. This will end up querying for ALL posts in the database and then fetching the corrections for each of them

@Takshil-Kunadia

Takshil-Kunadia commented Mar 17, 2025

Copy link
Copy Markdown
Collaborator Author

@leogermani Ahhh! Missed that there is no post-type check in that query. makes sense. will add that and remove the meta query. That should do it.

Gotcha! hmmm that would be a tad bit difficult! We can do a DB query to find unique post_id meta values from all the corrections posts. If its alright!

Post ID is saved in each correction

const CORRECTION_POST_ID_META = 'newspack_correction-post-id';

@Takshil-Kunadia

Copy link
Copy Markdown
Collaborator Author

@leogermani DB query to reverse search the post IDs of the corrections.

$post_ids = $wpdb->get_col(
	$wpdb->prepare(
		"SELECT DISTINCT meta_value FROM $wpdb->postmeta WHERE meta_key = %s",
		self::CORRECTION_POST_ID_META
	)
);

@Takshil-Kunadia

Copy link
Copy Markdown
Collaborator Author

@leogermani Please have a look at e8898f6

@leogermani leogermani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Takshil-Kunadia I actually had a closer look and made a call to remove the shortcode. I think this came from the old plugin we used as a base, but we don't need. That looked like a ticking bomb because it was querying all corrections from all posts... we don't need that anywhere now that we have a proper archive in the theme and the blocks for block themes

@github-actions github-actions Bot added the [Status] Approved The pull request has been reviewed and is ready to merge label Mar 18, 2025
@Takshil-Kunadia

Copy link
Copy Markdown
Collaborator Author

Thanks! @leogermani that takes care of it! 🙌

@Takshil-Kunadia Takshil-Kunadia merged commit 6a7cd45 into trunk Mar 18, 2025
@Takshil-Kunadia Takshil-Kunadia deleted the fix/corrections-location branch March 18, 2025 14:12
@github-actions

Copy link
Copy Markdown

Hey @Takshil-Kunadia, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

matticbot pushed a commit that referenced this pull request Mar 20, 2025
# [6.2.0-alpha.1](v6.1.1...v6.2.0-alpha.1) (2025-03-20)

### Bug Fixes

* **ga:** check if post is not null before reading properties ([#3817](#3817)) ([9dfa47a](9dfa47a))
* **modal-checkout:** endpoint to refresh newsletter lists via REST ([#3841](#3841)) ([79ea458](79ea458))
* **reader-revenue:** fatal when using woocommerce-paypal-payments@3.0.0 ([#3848](#3848)) ([1d73e17](1d73e17))
* update paths to share button, social CSS in Perfmatters ([#3810](#3810)) ([eb2ca7e](eb2ca7e))

### Features

* add Lite sites feature ([#3807](#3807)) ([cbd3e61](cbd3e61))
* **corrections-location:** integrate Correction location logic ([#3829](#3829)) ([6a7cd45](6a7cd45))
* enable email change for newspack users ([#3824](#3824)) ([1bfb458](1bfb458))
* **media:** enable setting a default image for 404 images ([#3811](#3811)) ([465928e](465928e))
* **memberships:** status reevaluation ([#3845](#3845)) ([ba4bea9](ba4bea9))
* **my-account:** disable WC password nag ([a22e756](a22e756))
* **rss-feed:** add feed enhancements for republication tracker; Atom feed URL ([#3801](#3801)) ([719983f](719983f))
* **woo-member-commenting:** optional module for member commenting ([#3783](#3783)) ([262f8bf](262f8bf))

### Reverts

* Revert "refactor(corrections): remove corrections feature flag (#3797)" (#3825) ([f5f6a5c](f5f6a5c)), closes [#3797](#3797) [#3825](#3825)
@matticbot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 6.2.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

dkoo added a commit that referenced this pull request Mar 31, 2025
* feat(my-account): disable WC password nag

* feat(woo-member-commenting): optional module for member commenting (#3783)

* feat(woo-member-commenting): optional module for member commenting

* feat(woo-comments): use a constant array for settings

* Remove punctuation

Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>

---------

Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>

* fix: update paths to share button, social CSS in Perfmatters (#3810)

* feat(media): enable setting a default image for 404 images (#3811)

* fix(ga): check if post is not null before reading properties (#3817)

* feat(rss-feed): add feed enhancements for republication tracker; Atom feed URL (#3801)

* feat: add Lite sites feature (#3807)

* feat: add Lite sites feature

* feat: add lite site settings page

* feat(lite-site): update style

* fix: a few tweaks

* fix: i18n

* fix: option name

* feat: add hook to allow styles customizations

* fix: a few tweaks to the UI

* feat: i18n for strings

---------

Co-authored-by: Thomas Guillot <thomas@automattic.com>

* Fix: Guest Contributors Role Init for CLI and PHPUnit (#3828)

* fix: guest contributors role init for CLI (non-admin)

* fix: guest contributors role init: move line

* fix: guest contributors role init: phpcs cleanup

* Revert "refactor(corrections): remove corrections feature flag (#3797)" (#3825)

This reverts commit 7776957.

* feat: enable email change for newspack users (#3824)

* feat: enable email change for newspack users

* fix: check ff for non-a12s

* test: update JS tests (#3818)

* deps: update testing-library

* refator: prevent react warnings

* test: fix test failing after testing-library update

* feat(corrections-location): integrate Correction location logic (#3829)

* refactor(corrections): remove customizer settings for corrections location

* feat(corrections): add location metadata to corrections and update modal

* feat(corrections): separate corrections by location and update markup generation

* fix(tests): update correction tests

* refactor(corrections): simplify location check

* feat: update button label

* fix: clear empty space

* docs: update docs

* refactor: remove unused corrections active meta key

* feat: remove shortcode

* refactor: remove obsolete corrections shortcode tests

---------

Co-authored-by: Leo Germani <leogermani@automattic.com>

* fix(modal-checkout): endpoint to refresh newsletter lists via REST (#3841)

This PR fixes an issue where the newsletters modal was incorrectly showing all premium newsletters. We fix this by refreshing the newsletters lists post checkout.

* test: fix corrections test

* fix(reader-revenue): fatal when using woocommerce-paypal-payments@3.0.0 (#3848)

* feat(memberships): status reevaluation (#3845)

* feat(memberships): status reevaluation

* chore: fix linting

* chore(release): 6.2.0-alpha.1 [skip ci]

# [6.2.0-alpha.1](v6.1.1...v6.2.0-alpha.1) (2025-03-20)

### Bug Fixes

* **ga:** check if post is not null before reading properties ([#3817](#3817)) ([9dfa47a](9dfa47a))
* **modal-checkout:** endpoint to refresh newsletter lists via REST ([#3841](#3841)) ([79ea458](79ea458))
* **reader-revenue:** fatal when using woocommerce-paypal-payments@3.0.0 ([#3848](#3848)) ([1d73e17](1d73e17))
* update paths to share button, social CSS in Perfmatters ([#3810](#3810)) ([eb2ca7e](eb2ca7e))

### Features

* add Lite sites feature ([#3807](#3807)) ([cbd3e61](cbd3e61))
* **corrections-location:** integrate Correction location logic ([#3829](#3829)) ([6a7cd45](6a7cd45))
* enable email change for newspack users ([#3824](#3824)) ([1bfb458](1bfb458))
* **media:** enable setting a default image for 404 images ([#3811](#3811)) ([465928e](465928e))
* **memberships:** status reevaluation ([#3845](#3845)) ([ba4bea9](ba4bea9))
* **my-account:** disable WC password nag ([a22e756](a22e756))
* **rss-feed:** add feed enhancements for republication tracker; Atom feed URL ([#3801](#3801)) ([719983f](719983f))
* **woo-member-commenting:** optional module for member commenting ([#3783](#3783)) ([262f8bf](262f8bf))

### Reverts

* Revert "refactor(corrections): remove corrections feature flag (#3797)" (#3825) ([f5f6a5c](f5f6a5c)), closes [#3797](#3797) [#3825](#3825)

* feat: add custom check for media visibility (#3823)

* feat(corrections-modal): Improve Corrections Modal UX (#3835)

* feat(corrections-modal): enhance corrections modal UX with notices and card layout

* feat(corrections-modal): replace success notice with snackbar

* refactor(corrections-modal): replaced contained notice with global notice

* feat(corrections-modal): reorganise panels

* feat(corrections-modal): enforce correction saving

---------

Co-authored-by: Thomas Guillot <thomas@automattic.com>

* feat(corrections-modal): fix modal action buttons & state handling (#3852)

* feat(correction): Add Priority setting & refactor Block (#3844)

* feat(corrections): update correction box to have location settings

* feat(corrections-blocks): add variations for top and bottom correction box

* feat(correction-box): change default location to 'all' in block attributes

* feat(correction-box): upgrade variations and update icons

* refactor(corrections): change location meta to priority and update related logic

* refactor(correction-box): rename location setting to priority & update its logic & remove variations

* refactor(corrections-modal): rename location to priority and update related logic

* docs: update docs

---------

Co-authored-by: Leo Germani <leogermani@automattic.com>

* chore(release): 6.2.0-alpha.2 [skip ci]

# [6.2.0-alpha.2](v6.2.0-alpha.1...v6.2.0-alpha.2) (2025-03-25)

### Bug Fixes

* **reader-revenue:** fatal when using woocommerce-paypal-payments@3.0.0 ([#3848](#3848)) ([a3f6d6a](a3f6d6a))
* **woocommerce-email:** fix the subscription cancellation email ([fc1cc8c](fc1cc8c))

* fix: add check if product before gating content (#3850)

* chore(release): 6.2.0-alpha.3 [skip ci]

# [6.2.0-alpha.3](v6.2.0-alpha.2...v6.2.0-alpha.3) (2025-03-26)

### Bug Fixes

* **esp-sync:** account for wp errors in email change sync ([#3860](#3860)) ([d5d576d](d5d576d))

* feat: update custom bylines data structure (#3863)

* feat: information architecture (#3857)

Co-authored-by: Jared Rethman <jaredrethman@gmail.com>
Co-authored-by: Ron Chambers <116242607+ronchambers@users.noreply.github.com>
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
Co-authored-by: Rasmy Nguyen <raz@automattic.com>
Co-authored-by: Thomas Guillot <thomasguillot@users.noreply.github.com>
Co-authored-by: Laurel <laurel.fulford@automattic.com>

* feat: add canonical url to lite site single posts (#3865)

* chore(release): 6.2.0-alpha.4 [skip ci]

# [6.2.0-alpha.4](v6.2.0-alpha.3...v6.2.0-alpha.4) (2025-03-26)

### Bug Fixes

* add check if product before gating content ([#3850](#3850)) ([b9e385d](b9e385d))

### Features

* add canonical url to lite site single posts ([#3865](#3865)) ([471ad81](471ad81))
* add custom check for media visibility ([#3823](#3823)) ([c1d81dc](c1d81dc))
* **correction:** Add Priority setting & refactor Block ([#3844](#3844)) ([9232750](9232750))
* **corrections-modal:** fix modal action buttons & state handling ([#3852](#3852)) ([220d5d6](220d5d6))
* **corrections-modal:** Improve Corrections Modal UX ([#3835](#3835)) ([afc9844](afc9844))
* information architecture ([#3857](#3857)) ([6fb5951](6fb5951))
* update custom bylines data structure ([#3863](#3863)) ([9b66c1f](9b66c1f))

* fix(ia): render all emails on reset (#3867)

This fixes an issue where resetting an email template in Newspack > Settings > Emails would only render Woo templates on success.

* chore(release): 6.2.0-alpha.5 [skip ci]

# [6.2.0-alpha.5](v6.2.0-alpha.4...v6.2.0-alpha.5) (2025-03-27)

### Bug Fixes

* **ia:** render all emails on reset ([#3867](#3867)) ([519890f](519890f))

* fix(image-404-handling): handle 404 from other sites

* chore(release): 6.2.0-alpha.6 [skip ci]

# [6.2.0-alpha.6](v6.2.0-alpha.5...v6.2.0-alpha.6) (2025-03-31)

### Bug Fixes

* **image-404-handling:** handle 404 from other sites ([51998b8](51998b8))

* test: fix unit tests based on dates relative to "now" (#3875)

* fix: handle missing Mailchimp API key in auth status (#3873)

* fix: remove Mailchimp for WooCommerce from wizard (#3876)

* chore(release): 6.2.0-alpha.7 [skip ci]

# [6.2.0-alpha.7](v6.2.0-alpha.6...v6.2.0-alpha.7) (2025-03-31)

### Bug Fixes

* handle missing Mailchimp API key in auth status ([#3873](#3873)) ([81a6e51](81a6e51))
* remove Mailchimp for WooCommerce from wizard ([#3876](#3876)) ([4353a98](4353a98))

* fix(notices): fix PHP notice (#3872)

---------

Co-authored-by: matticbot <sysops+ghmatticbot@automattic.com>
Co-authored-by: Adam Cassis <adam.cassis@automattic.com>
Co-authored-by: Camilla Krag Jensen <naxoc@users.noreply.github.com>
Co-authored-by: Laurel <laurel.fulford@automattic.com>
Co-authored-by: Adam Cassis <adam@adamcassis.com>
Co-authored-by: Eddie Carrasco <eddies.code.shop@gmail.com>
Co-authored-by: Takshil Kunadia <71006004+Takshil-Kunadia@users.noreply.github.com>
Co-authored-by: leogermani <leogermani@automattic.com>
Co-authored-by: Thomas Guillot <thomas@automattic.com>
Co-authored-by: Ron Chambers <116242607+ronchambers@users.noreply.github.com>
Co-authored-by: Rasmy Nguyen <raz@automattic.com>
Co-authored-by: matticbot <semantic-release-bot@martynus.net>
Co-authored-by: Miguel Peixe <miguel.peixe@automattic.com>
Co-authored-by: Jared Rethman <jaredrethman@gmail.com>
Co-authored-by: Thomas Guillot <thomasguillot@users.noreply.github.com>
matticbot pushed a commit that referenced this pull request Mar 31, 2025
# [6.2.0](v6.1.3...v6.2.0) (2025-03-31)

### Bug Fixes

* add check if product before gating content ([#3850](#3850)) ([b9e385d](b9e385d))
* **ga:** check if post is not null before reading properties ([#3817](#3817)) ([9dfa47a](9dfa47a))
* handle missing Mailchimp API key in auth status ([#3873](#3873)) ([81a6e51](81a6e51))
* **ia:** render all emails on reset ([#3867](#3867)) ([519890f](519890f))
* **image-404-handling:** handle 404 from other sites ([51998b8](51998b8))
* **modal-checkout:** endpoint to refresh newsletter lists via REST ([#3841](#3841)) ([79ea458](79ea458))
* **notices:** fix PHP notice ([#3872](#3872)) ([9cda1ca](9cda1ca))
* **reader-revenue:** fatal when using woocommerce-paypal-payments@3.0.0 ([#3848](#3848)) ([1d73e17](1d73e17))
* remove Mailchimp for WooCommerce from wizard ([#3876](#3876)) ([4353a98](4353a98))
* update paths to share button, social CSS in Perfmatters ([#3810](#3810)) ([eb2ca7e](eb2ca7e))

### Features

* add canonical url to lite site single posts ([#3865](#3865)) ([471ad81](471ad81))
* add custom check for media visibility ([#3823](#3823)) ([c1d81dc](c1d81dc))
* add Lite sites feature ([#3807](#3807)) ([cbd3e61](cbd3e61))
* **correction:** Add Priority setting & refactor Block ([#3844](#3844)) ([9232750](9232750))
* **corrections-location:** integrate Correction location logic ([#3829](#3829)) ([6a7cd45](6a7cd45))
* **corrections-modal:** fix modal action buttons & state handling ([#3852](#3852)) ([220d5d6](220d5d6))
* **corrections-modal:** Improve Corrections Modal UX ([#3835](#3835)) ([afc9844](afc9844))
* enable email change for newspack users ([#3824](#3824)) ([1bfb458](1bfb458))
* information architecture ([#3857](#3857)) ([6fb5951](6fb5951))
* **media:** enable setting a default image for 404 images ([#3811](#3811)) ([465928e](465928e))
* **memberships:** status reevaluation ([#3845](#3845)) ([ba4bea9](ba4bea9))
* **my-account:** disable WC password nag ([a22e756](a22e756))
* **rss-feed:** add feed enhancements for republication tracker; Atom feed URL ([#3801](#3801)) ([719983f](719983f))
* update custom bylines data structure ([#3863](#3863)) ([9b66c1f](9b66c1f))
* **woo-member-commenting:** optional module for member commenting ([#3783](#3783)) ([262f8bf](262f8bf))

### Reverts

* Revert "refactor(corrections): remove corrections feature flag (#3797)" (#3825) ([f5f6a5c](f5f6a5c)), closes [#3797](#3797) [#3825](#3825)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released on @alpha [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants