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

ANDAUTH-30 rewrite to retrofit #13

Merged
merged 30 commits into from May 30, 2018

Conversation

Projects
None yet
3 participants
@InsanusMokrassar
Contributor

InsanusMokrassar commented May 29, 2018

This task provide solution for ANDAUTH-30

  1. Rewrite work with all API to use retrofit (some methods, for example, login, have additional handle layer in XWikiHttp for extracting authtoken)
  2. Was removed all classes, which contains low-level work with sockets/network
  3. Sign up was temporary removed for the reason that now XWiki doesn't provide API for registration. In this case we could use some parsers, but I think, that will be better if for signing up we will redirect user to browser
  4. Fixed issue with synchronization of contacts

@InsanusMokrassar InsanusMokrassar requested review from tmortagne and pa1pal May 29, 2018

@pa1pal

Good job on implementing the apis using retrofit, deleting unused files and placing TODO for future reference. 👍

I tested the app, through it shows sync error, will test again after some time. Changes are large, I added some comments, will look it again.

I found very few inline comments, it would be better if there will be more doc.

// Lets give another try to authenticate the user
if (TextUtils.isEmpty(authToken)) {
if (TextUtils.isEmpty(authToken[0])) {

This comment has been minimized.

@pa1pal

pa1pal May 29, 2018

Member

Why Auth token array?
though you used static [0] index everywhere.

This comment has been minimized.

@InsanusMokrassar

InsanusMokrassar May 29, 2018

Contributor

Because final variables can't be modified, but items of final array can be modified:)

refreshAllAuthTokenType(am, account, authToken[0]);
} catch (InterruptedException e) {
Bundle result = new Bundle();
result.putString(AccountManager.KEY_ERROR_MESSAGE, "getAuthToken error impossible !!!");

This comment has been minimized.

@pa1pal

pa1pal May 29, 2018

Member

add the message in string xml

This comment has been minimized.

@InsanusMokrassar

InsanusMokrassar May 29, 2018

Contributor

I think that in separate PR I will implement fully work with strings, because this place is not alone

);
@GET(
ApiEndPoints.REST +

This comment has been minimized.

@pa1pal

pa1pal May 29, 2018

Member

clean implementation, Good job.

This comment has been minimized.

@InsanusMokrassar

InsanusMokrassar May 29, 2018

Contributor

Thank you

android:id="@+id/list_view"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:divider="#b5b5b5"

This comment has been minimized.

@pa1pal

pa1pal May 29, 2018

Member

Always add dimensions, colors and strings in their respective files.

@@ -52,16 +46,44 @@ public static void createService() {
.addInterceptor(interceptor)
.build();
// Check that url ends with `/` and put it if not
if (!baseUrl.endsWith("/")) {

This comment has been minimized.

@pa1pal
@InsanusMokrassar

This comment has been minimized.

Contributor

InsanusMokrassar commented May 29, 2018

I found very few inline comments, it would be better if there will be more doc.

I can implement it now or in additional commits after merge

InsanusMokrassar added some commits May 26, 2018

ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* remove old login method and rewrite reAuthToken where it try to reauth
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* rewrite getting of groups to use retrofit
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* remove XWikiHttp#availableGroups, change XWikiServices#availableGroups
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* remove redundant checking last modify of contact info (it called all info of user and get only modified field)
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* rewrite all to use actual format with properties
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* small refactoring/optimisation of sync settings
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* Add XWikiPhotosManager, rewrite usage of photos downloads on OkHttpClient in XWikiPhotosManager
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* remove redundant XWikiHttp#updateUser (replaced by XWikiServices#updateUser
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* update mechanism of creating/updating/deleting contacts from/into db
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* fix for last refactoring of user updating in database
ANDAUTH-30: Rewrite low-level usable of HTTP requests by Retrofit
* add handling of unfortunate getting data in sync settings
@pa1pal

pa1pal approved these changes May 30, 2018

not getting sync error for now. 👍

@tmortagne tmortagne merged commit 568082d into master May 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment