Login by evgenysuetin · Pull Request #10 · Kilograpp/Mattermost-Android-Native

evgenysuetin · 2017-02-10T11:46:05Z

create sqlite logic for login and choose team activity

Keparlg · 2017-02-10T11:59:18Z

+            values.put(FIELD_NAME, team.getDisplayName());
+            list.add(values);
+        }
+        ContentValues[] contentValues = new ContentValues[teams.size()];


Здесь в принципе можно не создавать новый массив, а прямо в вызове метода прописать list.toArray()

Keparlg · 2017-02-10T12:02:43Z

+
+    @Override
+    public boolean onOptionsItemSelected(MenuItem item) {
+        finish();


Сделай проверку на то, что мы нажимаем именно home button.

kraftuK · 2017-02-10T12:03:49Z

+        }
+        db.setTransactionSuccessful();
+        db.endTransaction();
+        getContext().getContentResolver().notifyChange(uri, null);


а почему не count?

kraftuK · 2017-02-10T12:05:45Z

+                .getApplicationContext()
+                .getContentResolver()
+                .bulkInsert(MattermostContentProvider.CONTENT_URI_TEAMS, contentValues);
+    }


MattermostApp.getSingleton().getApplicationContext()
Может сократить длину? каждому репозиторию добавить метод по умолчанию, который будет возвращать нужный контекст?

Keparlg · 2017-02-10T12:04:42Z

+        holder.bindTo(mCursor);
+        holder.itemView.setOnClickListener(view ->{
+            if(mCursor.moveToPosition(position)){
+                onItemClickListener.onItemClick(mCursor.getString(mCursor.getColumnIndex(DBHelper.FIELD_COMMON_ID)));


Да, я понял, о чем ты говорил. Чтобы получить одно поле, надо дофига чего написать. Действительно тогда будет лучше это завернуть в статические методы соответствующих классов

Это пока не точно, так как пока не известна точная модель данных.

kraftuK

Как то так

kraftuK · 2017-02-10T12:07:41Z

+                        null,
+                        null,
+                        null,
+                        FIELD_NAME);


Почему sortOrder передаем только название поле? Вроде нужно указывать по возрастанию или убыванию.

kraftuK · 2017-02-10T12:08:17Z

+        values.put(FIELD_LAST_PICTURE_UPDATE, user.getLastPictureUpdate());
+        values.put(FIELD_STATUS, user.getStatus());
+       /* values.put(FIELD_IN_TEAM, user.getInTeam());
+        values.put(FIELD_IS_SHOW, user.get());*/


Почему поля в комментарии?

kraftuK · 2017-02-10T12:08:59Z

+
+    @Override
+    public Loader onCreateLoader(int id, Bundle args) {
+        return new TeamsCursorLoader(getApplicationContext());


тут малость не правильно, т.к id не обрабатывается.

Я думаю мы пока решили вопрос с @kraftuK

kraftuK · 2017-02-10T12:09:15Z

+
+    @Override
+    public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
+        teamListAdapter.swapCursor(data);


проверку нужно сделать teamListAdapter может быть null

kraftuK · 2017-02-10T12:10:19Z


-        RealmResults<Team> teams = TeamRepository.query();
-        TeamListAdapter teamListAdapter = new TeamListAdapter(this, teams, this::setTeam);
+        teamListAdapter = new TeamListCursorAdapter(this, null,this::setTeam);


Я думаю нужно добавить конструктор, который который null скроет.

@kraftuK ну вот зато без такого конструктора я точно вижу, что в начале мы туда передаем курсор, но "пустой". На мой взгляд читаемость лучше.

kraftuK · 2017-02-10T12:13:55Z

+        holder.bindTo(mCursor);
+        holder.itemView.setOnClickListener(view ->{
+            if(mCursor.moveToPosition(position)){
+                onItemClickListener.onItemClick(mCursor.getString(mCursor.getColumnIndex(DBHelper.FIELD_COMMON_ID)));


нужно проверить листенера, мб null будет

нужно эту логику перенести внутрь холдера

kraftuK · 2017-02-10T12:14:28Z

+        } else {
+            return 0;
+        }
+    }


Нужно заменить на, будет красивее ()?():() =)

kraftuK · 2017-02-10T12:14:46Z

+        }
+        this.mCursor = newCursor;
+        notifyDataSetChanged();
+    }


тут не нужны this

kraftuK · 2017-02-10T12:15:20Z

+    public void bindTo(Cursor mCursor) {
+        if (mCursor.moveToPosition(getAdapterPosition())) {
+            String displayName = mCursor.getString(mCursor.getColumnIndex(TeamsRepository.FIELD_NAME));
+            if (displayName.length() != 0) {


Используй TextUntils.isEmpty

kraftuK · 2017-02-10T12:15:48Z

+        }
+    }
+
+}


Логику связки view и данные перенес бы viewholder

evgenysuetin · 2017-02-10T13:38:22Z

@kraftuK @Keparlg смотрите последие изменения

Keparlg · 2017-02-10T13:58:52Z

    private static final int CODE_TEAMS_WITH_ID = 6;
    private static final int CODE_USERS = 7;
    private static final int CODE_USERS_WITH_ID = 8;
+    private static final int CODE_PREFERENCES = 9;


А варианта с указанием Id не будет или впадлу писать?)

Не будет

Keparlg · 2017-02-10T13:59:39Z

+            values.put(FIELD_VALUE, preferences.getValue());
+            list.add(values);
+        }
+        ContentValues[] contentValues = new ContentValues[preferencesList.size()];


Здесь также можно обойтись без создания массива

согласен, поправим

evgenysuetin added 3 commits February 9, 2017 14:55

add sqlite to login screen

7199a01

begin add sqlite to chose team screen

2a00b63

add click to team list item

2bf3e04

evgenysuetin requested review from IvanProkofyev and Keparlg February 10, 2017 11:46

IvanProkofyev approved these changes Feb 10, 2017

View reviewed changes

Keparlg reviewed Feb 10, 2017

View reviewed changes

kraftuK reviewed Feb 10, 2017

View reviewed changes

Keparlg reviewed Feb 10, 2017

View reviewed changes

kraftuK reviewed Feb 10, 2017

View reviewed changes

review fix, add logic for save preferences

7c65ce0

IvanProkofyev approved these changes Feb 10, 2017

View reviewed changes

Keparlg reviewed Feb 10, 2017

View reviewed changes

+                      }
+                  }
+              }

Uh oh!

Conversation

evgenysuetin commented Feb 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evgenysuetin Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraftuK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evgenysuetin commented Feb 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

evgenysuetin Feb 10, 2017 •

edited

Loading