-
Notifications
You must be signed in to change notification settings - Fork 1.3k
OkHttpJsonApi#getMediaList migrated to retrofit #3054
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
OkHttpJsonApi#getMediaList migrated to retrofit #3054
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ A review job has been created and sent to the PullRequest network.
@ilgazer you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add tests to make sure this change works as expected?
Codecov Report
@@ Coverage Diff @@
## backend-overhaul #3054 +/- ##
===================================================
+ Coverage 4.71% 4.94% +0.23%
===================================================
Files 261 260 -1
Lines 12195 12153 -42
Branches 1043 1038 -5
===================================================
+ Hits 575 601 +26
+ Misses 11581 11507 -74
- Partials 39 45 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks great. Thanks for helping us with the migration. :)
@@ -25,6 +25,6 @@ public CategoryImageController(OkHttpJsonApiClient okHttpJsonApiClient) { | |||
* @return | |||
*/ | |||
public Single<List<Media>> getCategoryImages(String categoryName) { | |||
return okHttpJsonApiClient.getMediaList("category", categoryName); | |||
return null;// okHttpJsonApiClient.getMediaList("category", categoryName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilgazer Can you update this. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
That class is no longer used and will be deleted. Thanks for catching that.
6 Temmuz 2019 Cumartesi tarihinde Vivek Maskara <notifications@github.com>
yazdı:
… ***@***.**** commented on this pull request.
The PR looks great. Thanks for helping us with the migration. :)
------------------------------
In app/src/main/java/fr/free/nrw/commons/category/
CategoryImageController.java
<#3054 (comment)>
:
> @@ -25,6 +25,6 @@ public CategoryImageController(OkHttpJsonApiClient okHttpJsonApiClient) {
* @return
*/
public Single<List<Media>> getCategoryImages(String categoryName) {
- return okHttpJsonApiClient.getMediaList("category", categoryName);
+ return null;// okHttpJsonApiClient.getMediaList("category", categoryName);
@ilgazer <https://github.com/ilgazer> Can you update this. :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3054?email_source=notifications&email_token=AB2EXIYOTIFEXHJSLEZKBPLP6AX73A5CNFSM4H6NLV3KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5UZRQY#pullrequestreview-258578627>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB2EXI7AUHSMKCLOYUHTVNTP6AX73ANCNFSM4H6NLV3A>
.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works perfectly for me. There's just 1 issue.
- search for something for eg, "mango"
- search the categories tab
- select a category from this tab
- media is not found.
The issue is that the category passed is not suffixed with "Category:".
But i believe the issue is not related to this PR. So we can fix it separately.
* Migrated OkHttpJsonApi#getMediaList partially to MediaClient#getCategoryImages * Migrated rest of OkHttpJsonApi#getMediaList functionality to MediaClient#getCategoryImages * Removed unused code and tests * Fixed small bug * Added tests * Removed unused CategoryImageController
Maybe we should set a standart on whether we should pass around page names with namespace or not throughout the app. |
* Migrated OkHttpJsonApi#getMediaList partially to MediaClient#getCategoryImages * Migrated rest of OkHttpJsonApi#getMediaList functionality to MediaClient#getCategoryImages * Removed unused code and tests * Fixed small bug * Added tests * Removed unused CategoryImageController
* Migrated OkHttpJsonApi#getMediaList partially to MediaClient#getCategoryImages * Migrated rest of OkHttpJsonApi#getMediaList functionality to MediaClient#getCategoryImages * Removed unused code and tests * Fixed small bug * Added tests * Removed unused CategoryImageController
* Migrated OkHttpJsonApi#getMediaList partially to MediaClient#getCategoryImages * Migrated rest of OkHttpJsonApi#getMediaList functionality to MediaClient#getCategoryImages * Removed unused code and tests * Fixed small bug * Added tests * Removed unused CategoryImageController
…Client (#3056) * With media client APIs migrated to retrofit (#2998) * With media client APIs migrated to retrofit * Add test cases and java docs * Fix test * Fix build * Fix build and other minor issues * Fix tests * Categories related client API's migrated to retrofit (#3053) * Commit 1 * searchCategories migrated to retrofit * SearchCategoriesFragment migrated to new API * Removed unused code * Created tests * implemented searching by prefix fixed SearchCategoryFragment behaviour where the same categories would be added to the list instead of new ones. * added tests * Migrated searchTitles to searchCategories, function behaviour seems identical * With media client APIs migrated to retrofit (#2998) * With media client APIs migrated to retrofit * Add test cases and java docs * Fix test * Fix build * Fix build and other minor issues * Fix tests * Categories related client API's migrated to retrofit (#3053) * Commit 1 * searchCategories migrated to retrofit * SearchCategoriesFragment migrated to new API * Removed unused code * Created tests * implemented searching by prefix fixed SearchCategoryFragment behaviour where the same categories would be added to the list instead of new ones. * added tests * Migrated searchTitles to searchCategories, function behaviour seems identical * OkHttpJsonApi#getMediaList migrated to retrofit (#3054) * Migrated OkHttpJsonApi#getMediaList partially to MediaClient#getCategoryImages * Migrated rest of OkHttpJsonApi#getMediaList functionality to MediaClient#getCategoryImages * Removed unused code and tests * Fixed small bug * Added tests * Removed unused CategoryImageController * getSubCategoryList and getParentCategoryList migrated to retrofit (#3055) * Migrated getSubCategoryList to retrofit * Migrated getParentCategoryList to retrofit * Removed obsolete functions * Added tests * Fixed small bugs * Migrated OkHttpJsonApiClient#getMedia and getPictureOfTheDay to MediaClient * Removed obsolete functions and added tests * Fixed merge errors * With media client APIs migrated to retrofit (#2998) * With media client APIs migrated to retrofit * Add test cases and java docs * Fix test * Fix build * Fix build and other minor issues * Fix tests * Categories related client API's migrated to retrofit (#3053) * Commit 1 * searchCategories migrated to retrofit * SearchCategoriesFragment migrated to new API * Removed unused code * Created tests * implemented searching by prefix fixed SearchCategoryFragment behaviour where the same categories would be added to the list instead of new ones. * added tests * Migrated searchTitles to searchCategories, function behaviour seems identical * OkHttpJsonApi#getMediaList migrated to retrofit (#3054) * Migrated OkHttpJsonApi#getMediaList partially to MediaClient#getCategoryImages * Migrated rest of OkHttpJsonApi#getMediaList functionality to MediaClient#getCategoryImages * Removed unused code and tests * Fixed small bug * Added tests * Removed unused CategoryImageController * getSubCategoryList and getParentCategoryList migrated to retrofit (#3055) * Migrated getSubCategoryList to retrofit * Migrated getParentCategoryList to retrofit * Removed obsolete functions * Added tests * Fixed small bugs * Consume login client from data client library (#2894) Fix actions for review client Use data client library for notifications With delete helper migrated to data client With wikidata edits With notifications and modifications migrated to data client With upload migrated to retrofit Delete unused code Reuse thank interface from the library
This PR migrates OkHttpJsonApi#getMediaList to 2 different functions, MediaClient#getMediaListFromSearch and MEdiaClient#getMediaListFromCategory.