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

Host/Domain change support added for local avatar url #216

Merged
merged 16 commits into from Sep 25, 2023
Merged

Conversation

jayedul
Copy link
Contributor

@jayedul jayedul commented Jun 26, 2023

Description of the Change

Current domain/host will be used for local avatar URLs even after site migration.

Closes #64

How to test the Change

  • Simply edit the image url host in simple_local_avatar user meta, maybe in database directly for easy test.
  • Reload the page wherever the avatar image is supposed to load
  • The image should be regenerated for that specific avatar size and the URL should get back to its original state based on current host/domain.

Changelog Entry

Fixed - Local avatar urls remain old after domain/host change

Credits

Props @jayedul

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@jayedul jayedul self-assigned this Jun 26, 2023
@jeffpaul jeffpaul added this to the 2.7.6 milestone Jul 10, 2023
@jeffpaul jeffpaul requested review from a team and ravinderk and removed request for a team July 10, 2023 21:04
@ravinderk
Copy link
Contributor

@jayedul Is this pull request ready for review?

@jeffpaul
Copy link
Member

@ravinderk let's go ahead and proceed with review on the PR

Copy link
Contributor

@ravinderk ravinderk left a comment

Choose a reason for hiding this comment

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

@jayedul I left one suggestion. Let me know if you have questions.

includes/class-simple-local-avatars.php Show resolved Hide resolved
@jeffpaul jeffpaul marked this pull request as ready for review September 19, 2023 13:16
@github-actions github-actions bot added the needs:code-review This requires code review. label Sep 19, 2023
@ravinderk ravinderk self-requested a review September 19, 2023 13:25
@jeffpaul
Copy link
Member

@faisal-alvi can you help with the PHPUnit failures here as well as general testing/code review to help get this ready for merge/release?

Fatal error: Uncaught TypeError: array_key_exists(): Argument #2 ($array) must be of type array, string given in /var/www/html/wp-content/plugins/simple-local-avatars/includes/class-simple-local-avatars.php:342
@faisal-alvi
Copy link
Member

The PHPUnit and Cypress test failures are fixed.

Also, I've tested this in a single setup site, and it's working perfectly. I'd like to move forward with testing it in a multisite network setup before we proceed with the merge. Hopefully, I'll have some time next week to conduct that testing.

Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

It was tested in a multisite network, working as expected. Thanks for the work, everyone!

@faisal-alvi faisal-alvi removed the needs:code-review This requires code review. label Sep 25, 2023
@faisal-alvi
Copy link
Member

Note: Unable to merge, requires one more approval. cc @ravinderk

@ravinderk
Copy link
Contributor

@faisal-alvi pr approved 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avatar support when moving hosts / changing domains
4 participants