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

Optimize the process of deleting AMP data during uninstallation #6840

Merged
merged 17 commits into from
Jan 25, 2022

Conversation

dhaval-parekh
Copy link
Collaborator

@dhaval-parekh dhaval-parekh commented Jan 17, 2022

Summary

Fixes #6826

This PR optimized the process of deleting AMP data during uninstallation. Instead of deleting posts and terms one by one with the WordPress function, it will run the DELETE query directly to the database to delete records.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@dhaval-parekh dhaval-parekh self-assigned this Jan 17, 2022
@dhaval-parekh dhaval-parekh marked this pull request as ready for review January 17, 2022 14:12
@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2022

Plugin builds for ab388a3 are ready 🛎️!

@dhaval-parekh dhaval-parekh changed the title Bug/6826 delete data while uninstalling Optimize the process of deleting AMP data during uninstallation. Jan 17, 2022
@dhaval-parekh dhaval-parekh force-pushed the bug/6826-delete-data-while-uninstalling branch from a956171 to 25e2337 Compare January 20, 2022 07:38
@dhaval-parekh
Copy link
Collaborator Author

The broken unit test case is not related to changes in the current PR.
I will look into it and create a new PR.

@dhaval-parekh dhaval-parekh force-pushed the bug/6826-delete-data-while-uninstalling branch from 1dece18 to cf26652 Compare January 24, 2022 06:21
@dhaval-parekh
Copy link
Collaborator Author

The broken unit test case is not related to changes in the current PR. I will look into it and create a new PR.

This has been addressed 825a555 by Weston

Copy link
Collaborator

@milindmore22 milindmore22 left a comment

Choose a reason for hiding this comment

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

LGTM

@westonruter
Copy link
Member

For the sake of being exceedingly careful, I've added tests for each uninstall function to check that they individually are behaving as expected. So we have basically two sets of tests now that are checking and re-checking that uninstall is removing only the expected data and leaving everything else as-is.

@westonruter westonruter added this to the v2.2.1 milestone Jan 25, 2022
@westonruter westonruter changed the title Optimize the process of deleting AMP data during uninstallation. Optimize the process of deleting AMP data during uninstallation Jan 25, 2022
@westonruter westonruter merged commit d78bac5 into develop Jan 25, 2022
@westonruter westonruter deleted the bug/6826-delete-data-while-uninstalling branch January 25, 2022 05:36
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize : Plugin uninstallation with data
3 participants