Skip to content

Add a "Set as Featured Image" Button to Image Block - Refactor for Featured Image PostModel update#14521

Merged
jd-alexander merged 12 commits intogb/1011-set-featured-buttonfrom
gb/1011-set-featured-button-refactor
Jun 1, 2021
Merged

Add a "Set as Featured Image" Button to Image Block - Refactor for Featured Image PostModel update#14521
jd-alexander merged 12 commits intogb/1011-set-featured-buttonfrom
gb/1011-set-featured-button-refactor

Conversation

@jd-alexander
Copy link
Copy Markdown
Contributor

@jd-alexander jd-alexander commented Apr 23, 2021

Description

The main purpose of this PR is to refactor the logic that was being utilized to update the featured image id on the PostModel previously, we were utilizing the PostRepository that was within the EditPostSettingsFragment. This approach was deemed an area that needed improvement since, the Gutenberg side of things would be calling functionality within a fragment when the main point of entry is the EditPostActivity. I am also assuming there could have been app states where the fragment isn't created so this refactor ensures that the updates to the PostModel are done in an object that will be retained during the editing experience - EditPostActivity. Let us know what you think about the refactor done. Any improvements or ideas are more than welcomed.

To be merged into the following PR: #14503

Testing

The test cases below are from the main Gutenberg PR related to these changes. In addition, we should verify that the Tracks events edited/introduced with this PR work as expected.

Image Block - Set and Remove Featured Image

  1. In the app, navigate to HomeBlog Posts to create a new blog post, one which doesn't already have a featured image set.
  2. Add a new image block, and upload an image to it.
  3. Tap the image block's cog/gear icon to access its settings.
  4. Observe the new Set as Featured button at the bottom of the settings and tap it.
  5. Verify that you see a confirmation message about the newly set featured image and that the block now has a Featured banner overlaying it.
  6. Tap the image block's cog/gear icon again and this time select the Remove as Featured option.
  7. Verify that you see a confirmation message that the image has been removed and that the Featured banner is also removed.

Image Block - Tracks Verification

  1. After following the steps to set and remove an image as featured directly from the image block, navigate to the internal Tracks tool and choose the Live View option from the menu.
  2. Enter wpandroid_featured_image_picked_gutenberg_editor into the tool's Events field and then the username associated with the account you tested with in the Username field. Click Search.
  3. Verify that the event was logged correctly. Note, there is usually a delay of ~5 minutes before an event is logged in this tool.
  4. Use the same steps to search for the wpandroid_featured_image_removed_gutenberg_editor event, in order to verify that this event fires after the steps to remove a featured image via the editor were followed.

Post Settings - Set and Remove Featured Image

  1. Tap the three icons to the upper right of the post editor to access Post Settings.
  2. Tap existing featured image and then Remove.
  3. Tap the back button to confirm that the Featured banner has been removed from the featured image, as expected. Tap the cog/gear icon to then confirm the option to Set as Featured appears.
  4. Return to the Post Settings screen and set a new featured image.
  5. Tap the back button. If the new featured image hasn't already been added to the editor via an image block, add it.
  6. Confirm that the Featured banner displays overlays the correct image and that Remove as Featured appears in its settings.

Post Settings - Tracks Verification

  1. After following the steps to set a featured image via Post Settings, navigate to our internal Tracks tool and choose the Live View option from the menu.
  2. Enter wpandroid_featured_image_set_clicked_post_settings into the tool's Events field and then the username associated with the account you tested with in the Username field. Click Search.
  3. Verify that the event was logged correctly. Note, there is usually a delay of ~5 minutes before an event is logged in this tool.
  4. Use the same steps to search for the wpandroid_featured_image_remove_clicked_post_settings event, in order to verify that this event fires after the steps to remove a featured image via Post Settings were followed.

Regression Notes

  1. Potential unintended areas of impact
    The main unintended issues with these changes could be:
  • Modified track events aren't fired as expected.
  • The Post Settings featured image behavior gets broken.

To ensure that these areas aren't impacted we will test these paths thoroughly.

  1. What I did to test those areas of impact (or what existing automated tests I relied on)
    To test the areas of impact, the flows above were utilized.

  2. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Apr 23, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@jd-alexander jd-alexander requested a review from SiobhyB April 23, 2021 18:51
@jd-alexander jd-alexander added this to the 17.3 milestone Apr 23, 2021
Copy link
Copy Markdown
Contributor

@SiobhyB SiobhyB left a comment

Choose a reason for hiding this comment

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

@jd-alexander, this works perfectly for me once I add the @Inject annotation to the UpdateFeaturedImageUseCase.kt file. I added what I used to get this working as a suggestion as part of the review, let me know if this is correct or if there's a different syntax that'd be preferable.

I also submitted a couple of commits to fix small issues:

  • Changed a couple of remaining references from IMAGE_PICKED to IMAGE_PICKED_POST_SETTINGS : ce3785e
  • Removed unused imports from EditPostSettingsFragment.java: c1278d6

@JavonDavis JavonDavis modified the milestones: 17.3, 17.4 May 3, 2021
@JavonDavis JavonDavis modified the milestones: 17.4, 17.5 May 17, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented May 20, 2021

You can test the changes on this Pull Request by downloading the APK here.

@SiobhyB SiobhyB force-pushed the gb/1011-set-featured-button branch from 0eac50d to f0ff0ba Compare May 20, 2021 12:14
@SiobhyB SiobhyB force-pushed the gb/1011-set-featured-button-refactor branch 3 times, most recently from fec86e2 to d1e7eda Compare May 21, 2021 09:30
SiobhyB added 2 commits May 21, 2021 10:33
… Gutenberg editor

This commit adds a "IMAGE_REMOVED_GUTENBERG_EDITOR" event to complement the "IMAGE_PICKED_GUTENBERG_EDITOR" event, for when an image is removed via the image block.
@jd-alexander jd-alexander requested a review from ashiagr May 25, 2021 03:29
@jd-alexander jd-alexander marked this pull request as ready for review May 25, 2021 03:29
Copy link
Copy Markdown
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

Hey @jd-alexander 👋!

All works as described, good job 👍. Also from your regression notes, I've tested that the older track events for (featured image on post settings) are not broken, even though the event label got changed.

The only odd thing I noticed: UpdateFeaturedImageUseCase is formatted in commit 70dff2c but I still see it unformatted, can you re-check it? Feel free to merge after it.

@jd-alexander jd-alexander modified the milestones: 17.5, 17.6 May 28, 2021
@jd-alexander
Copy link
Copy Markdown
Contributor Author

All works as described, good job 👍. Also from your regression notes, I've tested that the older track events for (featured image on post settings) are not broken, even though the event label got changed.

Thanks much for the review @ashiagr it's appreciated 🙇🏾

The only odd thing I noticed: UpdateFeaturedImageUseCase is formatted in commit 70dff2c but I still see it unformatted, can you re-check it? Feel free to merge after it.

Thanks for spotting. I will do that!

@jd-alexander jd-alexander merged commit e81d2c2 into gb/1011-set-featured-button Jun 1, 2021
@jd-alexander jd-alexander deleted the gb/1011-set-featured-button-refactor branch June 1, 2021 04:51
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.

4 participants