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

Creating tests for Tribe__PUE__Notices class. #2332

Merged
merged 22 commits into from
Jan 9, 2025

Conversation

redscar
Copy link
Contributor

@redscar redscar commented Dec 20, 2024

🎫 Ticket

ET-2277

🗒️ Description

The original data corruption issue was caused by the use of array_merge_recursive, which allowed deeply nested and duplicated structures to accumulate in tribe_pue_key_notices. This PR resolves the issue by switching to wp_parse_args, which ensures that data is merged cleanly without creating unnecessary nesting.

  • Prevention of Corrupted Data:

    • Introduces enhanced sanitization and validation mechanisms.
    • Ensures that duplicate, deeply nested, or invalid data structures cannot be saved.
  • Repair of Existing Corrupted Data:

    • Detects and sanitizes previously saved corrupted data.
    • Flattens nested structures and removes invalid entries.
    • Retains only valid and necessary data to ensure integrity.
  • Performance and Stability Enhancements:

    • Reduces the risk of memory exhaustion errors caused by malformed data.
    • Improves the reliability and stability of the tribe_pue_key_notices option.

🎥 Artifacts

✔️ Checklist

  • Ran npm run changelog to add changelog file(s). More info here
  • Code is covered by NEW wpunit or integration tests.
  • Code is covered by EXISTING wpunit or integration tests.
  • Are all the required tests passing?
  • Automated code review comments are addressed.
  • Have you added Artifacts?
  • Check the base branch for your PR.
  • Add your PR to the project board for the release.

@redscar redscar requested a review from dpanta94 December 20, 2024 16:19
@redscar redscar self-assigned this Dec 20, 2024
@redscar redscar added the needs release Needs an associated release in Central before merging. label Dec 20, 2024
src/Tribe/PUE/Notices.php Outdated Show resolved Hide resolved
@redscar redscar changed the base branch from master to release/T25.imp January 6, 2025 16:37
@redscar redscar removed the needs release Needs an associated release in Central before merging. label Jan 6, 2025
@redscar redscar marked this pull request as ready for review January 8, 2025 18:41
@redscar redscar added the code review Status: requires a code review. label Jan 8, 2025
@redscar redscar requested a review from dpanta94 January 8, 2025 18:41
src/Tribe/PUE/Notices.php Outdated Show resolved Hide resolved
src/Tribe/PUE/Notices.php Outdated Show resolved Hide resolved
src/Tribe/PUE/Notices.php Outdated Show resolved Hide resolved
Eliminated unnecessary blank lines in the `remove_dismissed_notices` method to improve code readability. No functionality was affected by this change.
@redscar redscar requested a review from dpanta94 January 8, 2025 20:54
Copy link
Member

@dpanta94 dpanta94 left a comment

Choose a reason for hiding this comment

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

Some TBDs

src/Tribe/PUE/Notices.php Outdated Show resolved Hide resolved
src/Tribe/PUE/Notices.php Outdated Show resolved Hide resolved
Copy link
Member

@Camwyn Camwyn left a comment

Choose a reason for hiding this comment

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

A couple questions...

src/Tribe/PUE/Notices.php Show resolved Hide resolved
…ith direct instantiations of `Tribe__PUE__Notices`. Additionally, updated tests to align with the new approach and removed unnecessary setup and teardown methods.
@redscar redscar merged commit e51748b into release/T25.imp Jan 9, 2025
17 of 18 checks passed
@redscar redscar deleted the fix/PUE_Notices_duplication branch January 9, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code review Status: requires a code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants