Skip to content

Conversation

@ilgazer
Copy link
Contributor

@ilgazer ilgazer commented Jul 5, 2019

As per #3026 I'm migrating all category related client API's to retrofit.

  • searchCategory
  • searchCategories
  • allCategories

These 3 were merged into searchCategories as they were duplicates.

  • getSubCategoryList
  • getParentCategoryList

I'll have these ready tomorrow, but they are independent of searchCategories.

By the way, should I delete the mwApi originals?

Copy link

@pullrequest pullrequest bot left a 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.

Copy link

@tests-checker tests-checker bot left a 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-io
Copy link

codecov-io commented Jul 5, 2019

Codecov Report

Merging #3053 into backend-overhaul will increase coverage by 0.16%.
The diff coverage is 50%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           backend-overhaul   #3053      +/-   ##
===================================================
+ Coverage              4.55%   4.71%   +0.16%     
===================================================
  Files                   260     261       +1     
  Lines                 12261   12195      -66     
  Branches               1051    1043       -8     
===================================================
+ Hits                    558     575      +17     
+ Misses                11664   11581      -83     
  Partials                 39      39
Impacted Files Coverage Δ
...ons/explore/categories/SearchCategoryFragment.java 0% <0%> (ø) ⬆️
...rw/commons/mwapi/ApacheHttpClientMediaWikiApi.java 0% <0%> (ø) ⬆️
.../fr/free/nrw/commons/category/CategoriesModel.java 0% <0%> (ø) ⬆️
.../java/fr/free/nrw/commons/di/NetworkingModule.java 0% <0%> (ø) ⬆️
...a/fr/free/nrw/commons/category/CategoryClient.java 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33e9c75...fabf903. Read the comment docs.

@maskaravivek
Copy link
Contributor

@ilgazer Thanks for the PR. Nice work :)

I didn't understand this comment.

These 3 were merged into searchCategories as they were duplicates.

getCategoryImages
getSubCategoryList
getParentCategoryList

getCategoryImages returns a list of Media objects. Yes, getSubCategoryList and getParentCategoryList can probably be merged.

@ilgazer
Copy link
Contributor Author

ilgazer commented Jul 5, 2019 via email

return Observable.empty();
})
.map(MwQueryPage::title)
.doOnEach(s->Timber.d("Category returned: %s", s))
Copy link
Contributor

Choose a reason for hiding this comment

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

This logline can be removed before merging the PR. :)

@maskaravivek
Copy link
Contributor

I’m sorry for not being clear, the first 3 were merged into searchCategories. I’m going to work on getCategoryImages getSubCategoryList and getParentCategoryList today.

Okay, great. Ideally getCategoryImages can be a separate PR. And getSubCategoryList and getParentCategoryList can be in a single PR. And this PR can be independent of both these PRs. To me, it is good to be merged after 1 round of testing.

@maskaravivek
Copy link
Contributor

By the way, should I delete the mwApi originals?

Yes, feel free to delete the unused APIs. :)

@ilgazer
Copy link
Contributor Author

ilgazer commented Jul 5, 2019

I feel that those belong to the same CategoriesClient and CategoriesInterface. Would I be able to put them there while also having a separate PR?

@maskaravivek
Copy link
Contributor

I am testing this PR right now. If my tests work then we can merge this PR as it is in backend-overhual branch. You can probably start working on other APIs after that. :)

@ilgazer
Copy link
Contributor Author

ilgazer commented Jul 5, 2019

Ok, I'Il write unit tests in the meantime :)

@maskaravivek
Copy link
Contributor

Can you also remove unused code?

ilgazer added 3 commits July 5, 2019 16:11
fixed SearchCategoryFragment behaviour where the same categories would be added to the list instead of new ones.
@maskaravivek maskaravivek merged commit 52284b8 into commons-app:backend-overhaul Jul 5, 2019
@ilgazer ilgazer deleted the backend-overhaul-retrofit-categories branch July 6, 2019 08:47
maskaravivek pushed a commit to maskaravivek/apps-android-commons that referenced this pull request Jul 6, 2019
* 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
maskaravivek pushed a commit to maskaravivek/apps-android-commons that referenced this pull request Jul 7, 2019
* 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
maskaravivek pushed a commit to maskaravivek/apps-android-commons that referenced this pull request Jul 7, 2019
* 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
maskaravivek pushed a commit to maskaravivek/apps-android-commons that referenced this pull request Jul 10, 2019
* 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
maskaravivek pushed a commit that referenced this pull request Jul 15, 2019
…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
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