Add Tenor support #11587

ThomazFB · 2020-04-04T01:26:57Z

Summary

This pull request proposes a solution that brings a complete integration for the Tenor API within the pre-existent GIF selection code inside the WordPress application, solving the issue 11456.

This is an aggregation branch for four different branches:

Pull Request	Link
Tenor API integrated with GIF Picker	ThomazFB#1
GIF selection fully restored with Tenor API improvements	ThomazFB#2
Replace UI and internal Giphy references	ThomazFB#3
Add Tenor feature flag through selected build type	ThomazFB#4

Changelog

Tenor integration changes

A GifProvider interface is now the only coupling between all code for the GIF picker and the Tenor implementation, avoiding any reference to the Tenor library and making it easy to replace the Provider as we're doing now.
The TenorProvider implements the GifProvider interface and is injected within the DataSource via Dagger, this way only the AppplicationModule knows about the TenorProvider. Also, the TenorProvider is covered by unit tests in the TenorProviderTest class.
The already implemented pagination feature and list actions are all kept and integrated to work smoothly with the GifProvider, this integration happens through the available PositionalDataSource, The Tenor implementation makes sure that positioning tracking is respected.
The original integration with the MediaBrowserActivity was restored, making GIF selection available again.
The original integration with the EditPostActivity was restored, making GIF selection available again when writing or editing posts.
Analytics tracking restored.

UI changes

The Giphy attribution icon was removed and replaced by the Tenor attribution icon.
Every UI string with Giphy information was replaced with Tenor.

Refactoring and improvements changes

Test suite class added to GiphyPickerDataSource in order to better enforce the business logic when consuming any GifProvider implementation.
The giphy packages inside ui and viewmodel were both renamed to only gif.
All classes with the Giphy naming convention were renamed to only Gif to better fit the agnostic gif provider implementation strategy.
All comments and documentation with references to Giphy or the Giphy SDK were readapted to better fit the GifProvider structure.

Build configuration changes

The BuildConfig now receive the declaration of the boolean TENOR_AVAILABLE field directly from the build.gradle.
The button availability to select the Tenor GIF picker follows the TENOR_AVAILABLE value, if the value is false, the button doesn't get configured and should never appear on the screen.
The TENOR_AVAILABLE is set as true for the Wasabi and Zalpha builds and false for the Vanilla build.
Removed the Giphy SDK reference at maven repository in build.gradle.

Testing

Before doing any tests, make sure that to create a Tenor API key and paste it associated with the wp.tenor.api_key as the value inside your gradle.properties

GIF picking from Media Browser

How to test

Go to the Media Browser
Click on the Add button at the top right. Select the Choose from Tenor option
Type in the search bar to query GIFs
Once inside the Gif Picker, make sure that selection, preview and add are all working as expected

GIF picking from the Editor

How to test

Click to create a new post
Click on the overflow button at the top right end of the screen
select the Switch to classic editor
Click on the plus (+) button at the bottom left end of the screen
Select the Choose from Tenor option
Type in the search bar to query GIFs
Once inside the Gif Picker, make sure that selection, preview and add are all working as expected

Tenor GIF picker feature flag - Post Editor

Wasabi and Zalpha	Vanilla

How to test with Wasabi and Zalpha build

Click to create a new post
Click on the overflow button at the top right end of the screen
select the Switch to classic editor
Click on the plus (+) button at the bottom left end of the screen
Verify that the Choose from Tenor option is available
Type in the search bar to query GIFs
Once inside the Gif Picker, make sure that selection, preview and add are all working as expected

How to test with Vanilla build

Click to create a new post
Click on the overflow button at the top right end of the screen
select the Switch to classic editor
Click on the plus (+) button at the bottom left end of the screen
Verify that the Choose from Tenor option isn't there

Tenor GIF picker feature flag - Media Browser

Wasabi and Zalpha	Vanilla

How to test with Wasabi and Zalpha build

Go to the Media Browser
Click on the Add button at the top right
Verify that the Choose from Tenor option is available
Type in the search bar to query GIFs
Once inside the Gif Picker, make sure that selection, preview and add are all working as expected

How to test with Vanilla build

Go to the Media Browser
Click on the Add button at the top right.
Verify that the Choose from Tenor option isn't there

PR submission checklist

I have considered adding unit tests where possible.
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 · 2020-04-04T01:27:03Z

	Warnings
⚠️	PR is not assigned to a milestone.
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

ThomazFB · 2020-04-04T21:59:26Z

Hello @planarvoid! Just for your information, I've added four additional commits directly to this branch.

The c604372736bfb3366aab5ee46cdee3222bd73ebb to adjust the licenses.html file as we talked before.

The 74082b99a8612d026dcea26a65ea1520a17e67a7, e163ac84c6b444e2e1744750d4b67c89fc0296db and 009684b9e1e1e034aa4fd3af37a1c4933cefab6e to fix the checkstyle and lint reports.

Also, I've done some rebases with the develop branch to fix some conflicts.

What do you think?

planarvoid · 2020-04-06T14:33:36Z

Hey @ThomazFB, thanks for the PR! I was wondering about why you moved the error string from the strings.xml into a constant? This way it can't be translated. I actually quite liked the previous approach. Is it the Unused resource lint warning? We usually suppress these when the strings are used just in the code.

ThomazFB · 2020-04-06T14:47:30Z

Hey @ThomazFB, thanks for the PR! I was wondering about why you moved the error string from the strings.xml into a constant? This way it can't be translated. I actually quite liked the previous approach. Is it the Unused resource lint warning? We usually suppress these when the strings are used just in the code.

Hello @planarvoid! It was exactly because of the lint warning! The report was pointing out about the unused strings. And so I thought that the project was restricted about using strings for Exception messages instead of UI ones, should we suppress those errors then?

…ed to the PickerDataSourceFactory

…ewModel and GifPickerDataSource

…t cases

…dded more documentation

…tActivity

…owserActivity

…side Tenor

ThomazFB · 2020-04-07T03:16:37Z

Hey @planarvoid! Thank you for your feedback! Just letting you know that I committed a fix with your proposed string suppression for unused warnings and reverted the translatable string usage. Here's the commit: 0c09c88. What do you think?

edit: I've also done another rebase because of a new conflict with the develop branch.

planarvoid · 2020-04-07T08:24:50Z

thanks for the changes @ThomazFB, we usually do git merge develop instead of rebases here but rebase works just as well :-).
I'd recommend doing something like this to ignore the exception:

<string name="stats_site_not_loaded_yet" tools:ignore="UnusedResources">Site not loaded yet</string>

ThomazFB · 2020-04-07T12:33:31Z

Thank you for the suggestions @planarvoid! I'll prioritize merges instead of rebases then! Also, I've just modified the suppression declaration right here: 13e88ec. Let me know what you think.

planarvoid

Thanks for all the changes and the great PR description. It works nicely 👍

This dependency seems that was lastly used only for the no longer existing 'TenorProviderTest' test class (see 47ac89a), which was added as part of this #11587 PR. However, as part of this #14727 PR, this 'TenorProviderTest' test class got removed (see f29f6d0). As such, this leftover from back then can be now safely removed. PS: And with this change, the corresponding 'androidxTestCoreVersion' version is no longer necessary for unit test. As such, it is being moved from the more inclusive 'test' section to the more specific 'android test' section.

ThomazFB force-pushed the issue/11456-add-tenor-support branch from e0ce4cf to 009684b Compare April 4, 2020 03:07

ThomazFB force-pushed the issue/11456-add-tenor-support branch 4 times, most recently from 50086ea to 3b9a13d Compare April 4, 2020 23:00

planarvoid self-requested a review April 6, 2020 14:29

planarvoid assigned planarvoid and ThomazFB Apr 6, 2020

planarvoid added [Type] Enhancement [Type] Task labels Apr 6, 2020

ThomazFB added 16 commits April 6, 2020 23:26

Added Tenor library and API key reference

06f49e1

Changed giphy package name to gifs

be9e118

Created TenorProvider infrastructure and basic search

47ac89a

Added GifProvider and TenorProvider documentation

99751e6

GifProvider is now available through injection with Dagger and provid…

d393dca

…ed to the PickerDataSourceFactory

Replaced Giphy name to Gif from all packages and classes

c401c4c

Connected the TenorProvider implementation to the expected GifMediaVi…

6554d88

…ewModel and GifPickerDataSource

Improved test suite for TenorProvider with code refactor and more tes…

e376715

…t cases

Improved code reuse, naming and deprecated assertion methods

b2d093f

Gif data class removed, using the MutableGifMediaViewModel instead. A…

2e06912

…dded more documentation

Fixed GiphyPickerActivity reference on AndroidManifest

bddff06

Integrated pagination into the Tenor requests

a75cd47

Reverted Giphy renaming

91a00b2

Reverted Giphy renaming for packages

7e384e3

Additional code refactoring and new test for TenorProvider

5b1f835

Reconnected GIF search with MediaBrowserActivity

0d23a81

ThomazFB added 21 commits April 6, 2020 23:26

Refactored TenorProviderTest boilerplate

e3efa24

Removed unnecessary methods for response and error handling

1a62bb1

Replaced Tenor callback implementation to the default retrofit one

a854c91

Removed custom timeout exception implementation

4efed5f

Removed coroutine scope injection

ec38306

Remove unecessary annotation

bbf6419

Renamed all giphy name references inside viewmodel package to only gif

40a5873

Renamed all giphy name references inside ui package to only gif

3a41fcb

Replaced all Giphy string references to Tenor

b5b0af6

Adjusted all remaining documentation and code referencing Giphy

0ed1483

Adjust GifMediaViewModel documentation

af0aaa7

Removed maven reference for giphy SDK

35f8d0a

Removed translations of gif strings

93fe31c

Added local tenor feature flag through BuildConfig

a27b47a

Added Tenor selection availability through BuildConfig inside EditPos…

dfc607f

…tActivity

Added Tenor selection availability through BuildConfig inside MediaBr…

4a0cc13

…owserActivity

Added the Tenor availability to the alpha build

d512243

Added Tenor library license attribution

41cdfaf

Fixed checkstyle errors

f3bf4e3

Replaced UI string usage for Exception for internal constant string

2ad832a

Removed UI string usage from the exception for empty search result in…

ceac897

…side Tenor

ThomazFB force-pushed the issue/11456-add-tenor-support branch from 3b9a13d to ceac897 Compare April 7, 2020 02:35

Revert TenorProvider string usage with lint unused suppression

0c09c88

Changed the unused resources suppression declaration

13e88ec

planarvoid approved these changes Apr 8, 2020

View reviewed changes

planarvoid merged commit 80cc5c3 into wordpress-mobile:develop Apr 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Tenor support #11587

Add Tenor support #11587

ThomazFB commented Apr 4, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Apr 4, 2020 •

edited

Loading

ThomazFB commented Apr 4, 2020 •

edited

Loading

planarvoid commented Apr 6, 2020 •

edited

Loading

ThomazFB commented Apr 6, 2020 •

edited

Loading

ThomazFB commented Apr 7, 2020 •

edited

Loading

planarvoid commented Apr 7, 2020

ThomazFB commented Apr 7, 2020

planarvoid left a comment

Add Tenor support #11587

Add Tenor support #11587

Conversation

ThomazFB commented Apr 4, 2020 • edited Loading

Summary

Changelog

Tenor integration changes

UI changes

Refactoring and improvements changes

Build configuration changes

Testing

GIF picking from Media Browser

How to test

GIF picking from the Editor

How to test

Tenor GIF picker feature flag - Post Editor

How to test with Wasabi and Zalpha build

How to test with Vanilla build

Tenor GIF picker feature flag - Media Browser

How to test with Wasabi and Zalpha build

How to test with Vanilla build

PR submission checklist

peril-wordpress-mobile bot commented Apr 4, 2020 • edited Loading

ThomazFB commented Apr 4, 2020 • edited Loading

planarvoid commented Apr 6, 2020 • edited Loading

ThomazFB commented Apr 6, 2020 • edited Loading

ThomazFB commented Apr 7, 2020 • edited Loading

planarvoid commented Apr 7, 2020

ThomazFB commented Apr 7, 2020

planarvoid left a comment

Choose a reason for hiding this comment

ThomazFB commented Apr 4, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Apr 4, 2020 •

edited

Loading

ThomazFB commented Apr 4, 2020 •

edited

Loading

planarvoid commented Apr 6, 2020 •

edited

Loading

ThomazFB commented Apr 6, 2020 •

edited

Loading

ThomazFB commented Apr 7, 2020 •

edited

Loading