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

Make sure user ID exists #2964

Conversation

andrewlimaza
Copy link
Contributor

@andrewlimaza andrewlimaza commented Apr 18, 2024

  • BUG FIX: Fixes an issue where "Recent Members" dashboard widget would throw a warning for a recently deleted user as the code is cached and not cleared on user deletion.

This adds a final check before outputting the data to ensure that the user exists.

Another option would be to cache this for a shorter period in time or clear cache on user deletion.

All Submissions:

How to test the changes in this Pull Request:

  1. Ensure WP_DEBUG is enabled.
  2. Delete a member from the Users Table (but it has to be a recent member)
  3. Navigate to the dashboard widget "Recent Members" and see the error.
  4. Implement this code
  5. Re-view the Recent Member widget and see the user is gone. This is same functionality as once the transient is cleared.

See ticket #562793 for reference.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run tests with your changes locally?

Changelog entry

Enter a summary of all changes on this Pull Request. This will appear in the changelog if accepted.

* BUG FIX: Fixes an issue where "Recent Members" dashboard widget would throw a warning for a recently deleted user as the code is cached and not cleared on user deletion.

This adds a final check before outputting the data to ensure that the user exists.

Another option would be to cache this less aggressively which could be included in this same PR.
Copy link
Member

@dparker1005 dparker1005 left a comment

Choose a reason for hiding this comment

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

With this solution, deleted users may cause fewer than 5 members to be shown in the dashboard report. In the worst case, if all of the 5 most recent members were deleted, an empty table would be shown.

Would we want to consider an alternative approach where we could build the array of WP_User objects immediately after the line $theusers = get_transient( 'pmpro_dashboard_report_recent_members' );, and if any of those user objects does not exist, "break" the transient and get fresh data via SQL?

@andrewlimaza
Copy link
Contributor Author

I think that could work too, this was a solution for until the transient expires but I could see it going that way (or just caching it for a shorter period like 1 hour? which would reduce the chance of this happening.

Another solution would be to delete the transient on any user deletion?

@dparker1005
Copy link
Member

The more I think about this, the more I am on board with this quick fix. You're right, the transient will expire and then everything will be good. This PR avoids PHP errors/warnings, so is a huge improvement over the code already there. I like it

@dparker1005 dparker1005 merged commit 4cc6af0 into strangerstudios:dev Apr 18, 2024
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.

None yet

2 participants