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

Retire ENForm block #2466

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Retire ENForm block #2466

merged 1 commit into from
Dec 16, 2024

Conversation

comzeradd
Copy link
Member

@comzeradd comzeradd commented Dec 5, 2024

Summary

Removes all code exclusively required for the ENForm block.

In addition:

  • Removes relevant styles from Counter block
  • Removes relevant locale text domains
  • Removes twig templates that no other block uses
  • Adjust locale text domains
  • Added migration to clean up database from block settings and forms

Ref: https://jira.greenpeace.org/browse/PLANET-7673

Testing

  1. Before switching to this branch, enable Engaging Networks from Planet 4 > Features.
  2. Add some "random" keys into Engaging Networks > Settings. You probably already have some data there, since defaultcontent still does.
  3. Browse to Engaging Networks" > Add Form and create a new form. Same as above, you probably already have one as part of the defaultcontent database.
  4. Switch to this branch and build assets (npm run build:assets).
  5. Run the migration (npx wp-env run cli wp p4-run-activator)
  6. Verify it actually removed all relevant content:
    • There should be no feature_engaging_networks option in that array:
    npx wp-env run cli wp option get planet4_features
    • You should get an Error: Could not get 'p4en_main_settings' option. Does it exist?
    npx wp-env run cli wp option get p4en_main_settings
    • You should get an empty array:
    npx wp-env run cli wp post list --post_type=p4en_form
  7. In general, check code into detail in case something removed is still needed. I removed completely the block_templates and Controllers/API folders, since they don't seem to be used anywhere else besides ENForm block.

planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 5, 2024
/unhold cc304332-7b70-4c89-97b1-74d91aa9891d
planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 5, 2024
/unhold 3e98b51a-de70-4e7d-bda6-8309559bc612
@planet-4
Copy link
Contributor

planet-4 commented Dec 5, 2024

Test instance is ready 🚀

🌑 phobos | admin | blocks report | CircleCI | composer-local.json

⌚ 2024.12.16 09:12:24

planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 6, 2024
/unhold 2d010985-8a65-4817-916f-70a9bbb3b793
@comzeradd comzeradd self-assigned this Dec 9, 2024
@comzeradd comzeradd requested a review from mardelnet December 9, 2024 09:27
@comzeradd comzeradd marked this pull request as ready for review December 9, 2024 09:28
planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 9, 2024
/unhold 5a11095a-c978-4506-ac76-4011992c0380
planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 11, 2024
/unhold 2dada950-2745-4932-b7ba-1e76bf2d4bc7
Copy link
Contributor

@mardelnet mardelnet left a comment

Choose a reason for hiding this comment

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

I see there are still elements related to the ENForm in the plugin repo (check here and here, for example). Is it worth it to remove them, as well?

src/MasterSite.php Outdated Show resolved Hide resolved
src/Migrations/M001EnableEnFormFeature.php Show resolved Hide resolved
Copy link
Contributor

@mardelnet mardelnet left a comment

Choose a reason for hiding this comment

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

There's a conflict with the file tests/e2e/blocks/enform.spec.js that prevents this branch from being merged. Would you mind checking it?

@comzeradd
Copy link
Member Author

I see there are still elements related to the ENForm in the plugin repo (check here and here, for example). Is it worth it to remove them, as well?

These results are coming from the blocks plugin repo, so these are removed in the other PR: greenpeace/planet4-plugin-gutenberg-blocks#1265

planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 12, 2024
/unhold 5a8e12e0-cd96-434c-9b9a-a6b00493bcc7
planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 12, 2024
/unhold da9e6f1b-bbca-46fa-a7dc-f9e51f18ebda
planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 13, 2024
/unhold fe760b7c-aef8-4881-b02b-1a0f5b8f1a77
Copy link
Contributor

@mardelnet mardelnet left a comment

Choose a reason for hiding this comment

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

@comzeradd, for some reason, on my local installation the migration script removes all the ENForms except this one (ID 855):

Report-‹-Greenpeace-—-WordPress

src/Migrations/M036RemoveEnFormOptions.php Outdated Show resolved Hide resolved
Removes all code exclusively required for the ENForm block.

- Removes relevant styles from Counter block
- Removes relevant locale text domains
- Removes twig templates that no other block uses
- Adjust locale text domains
- Added migration to clean up database from block settings and forms
planet-4 added a commit to greenpeace/planet4-test-phobos that referenced this pull request Dec 16, 2024
/unhold 8b921cb3-ebc7-4e57-b5cb-32a17429f63f
@comzeradd
Copy link
Member Author

comzeradd commented Dec 16, 2024

@comzeradd, for some reason, on my local installation the migration script removes all the ENForms except this one (ID 855):

There was an error on the post type constants. It's looking up for action but the correct name of that post type is p4_action. I fixed it now in this PR and it removes even that instance of ENForm.

Copy link
Contributor

@mardelnet mardelnet left a comment

Choose a reason for hiding this comment

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

Well done!

@comzeradd comzeradd merged commit f6e7ac0 into main Dec 16, 2024
15 checks passed
@comzeradd comzeradd deleted the retire-enform branch December 16, 2024 15:29
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.

3 participants