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

Update hooks priority for wp_generate_attachment_metadata #271

Merged
merged 2 commits into from
Mar 22, 2021

Conversation

thrijith
Copy link
Member

  • Update hooks priority for wp_generate_attachment_metadata to work with azure storage
  • Move classifai_generate_image_alt_tags_source_url filter to helper function
  • Use get_modified_image_source_url where rescanning is done local file

Description of the Change

  • ClassifAI is generating tags on 10 priority, while the storage plugin uploads image to storage blob and deletes locally on priority 9, by the time it is getting there to generate tags, the local image is gone and alt text is not generated, reducing priority of the hook will allow it to generate the text and then upload to storage.

  • I found a related PR Fixes azure storage compatibility issue with classifai. #250 where the hook priority was increased due to similar issues, setting hook priority to be in following order smart_crop at 7 and alt_text at 8 should hopefully fix issues both issues.

  • The above changes only fix issue when the image is being uploaded, it doesn't work when we try to generate/re-generate/rescan image tags from media modal for an existing image, adding the filter introduced in Allow filtering of the image URL prior to generating image tags and alternate text #217 it other places will help with the same.

Verification Process

  • Tested on project that was facing this issue, applying these changes fixed the alt generation for images and image was successfully uploaded to Azure storage as well.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

  • Doesn't have one right now, happy to open one if needed.

…ith azure storage

Move `classifai_generate_image_alt_tags_source_url` filter to helper function
Use `get_modified_image_source_url` where rescanning is done local file
@thrijith thrijith requested a review from jeffpaul March 12, 2021 17:34
@jeffpaul jeffpaul added this to the 1.7.0 milestone Mar 12, 2021
@jeffpaul jeffpaul requested a review from dkotter March 12, 2021 17:35
dkotter
dkotter previously approved these changes Mar 15, 2021
@dkotter
Copy link
Collaborator

dkotter commented Mar 15, 2021

@thrijith Thanks for this PR. Code looks good and worked fine in my testing. There is a failed test though, if you can look at that and get it fixed up?

@thrijith
Copy link
Member Author

@thrijith Thanks for this PR. Code looks good and worked fine in my testing. There is a failed test though, if you can look at that and get it fixed up?

Thanks for the review @dkotter, the test failure seems unrelated to the change, I've updated the failing case it seems to be passing now, also added a change to fix some other alerts for risky tests.

@jeffpaul jeffpaul merged commit 883c6cc into develop Mar 22, 2021
@jeffpaul jeffpaul deleted the fix/alt-text-generation-conflict branch March 22, 2021 20:47
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.

3 participants