MM-35115 [Gekidou] Login flow - Email and Password #5402

avinashlng1080 · 2021-05-24T11:54:13Z

Summary

We are porting components from the current mobile app onto Gekidou.

This PR contains updated code for the first screen, login options screen and then a temporary placeholder for the channel screen.

Ticket Link

https://mattermost.atlassian.net/browse/MM-35115

NONE

…e [IN PROGRESS]

…[IN PROGRESS]

…PROGRESS]

…Url as well as the connection

* fix: add getMostRecentServerUrl func * fix: add ts and tsx to editorconfig * fix: rename functions * fix: return type

migbot · 2021-06-16T22:08:47Z

app/database/manager/index.ts

+      let connection: DatabaseInstance;
+
+      if (serverDatabase) {
+          // This serverUrl has previously been registered on the app
+          connection = serverDatabase.dbInstance;


Overall I'm confused with the various ways in which db related things are referred to. For example, the above retrieveDatabaseInstances doesn't really return database instances, but objects that have a property dbInstance which is the db instance. But then we call this property value a connection?

We are using different terms but each refer to the same thing. A databaseInstance is of type Database or undefined in case WDB was not able to create a connection to the *.db file. A connection is a valid pointer to a database file.

The retrieveDatabaseInstances method returns return {dbInstance, displayName, url}; if a matching url has been found under the Servers entity in the default database.

Imagine we have multi-server support. The user opens the side-bar to perform a server switch. Instead of returning an array of object, we were returning only the connections. I find it easier to identify connections using their url/displayName rather than retrieving it from inside the dbInstance object.

migbot · 2021-06-16T22:08:53Z

app/database/manager/index.ts

+     */
+  getDatabaseConnection = async ({serverUrl, setAsActiveDatabase, connectionName}: GetDatabaseConnectionArgs) => {
+      // We potentially already have this server registered; so we'll try to retrieve it if it is present under DEFAULT_DATABASE/GLOBAL entity
+      const existingServers = await this.retrieveDatabaseInstances([serverUrl]) as RetrievedDatabase[];


Nit: existingDatabases or existingServerDatabases (to be explicity about the default database not being included).

migbot · 2021-06-16T22:10:38Z

app/database/manager/index.ts

+          connection = serverDatabase.dbInstance;
+      } else {
+          // Or, it might be that the user has this server on the web-app but not mobile-app; so we'll need to create a new entry for this new serverUrl
+          const databaseName = connectionName ?? urlParse(serverUrl).hostname;


Should we be using urlParse(serverUrl).host + urlParse(serverUrl).path to ensure we get the port number and subdirectory, if any? For example, if we have serverUrl https://domain.com:1234/mattermost then we'd want the db name to be domain.com:1234/mattermost, right?

A name like domain.com:1234/mattermost is invalid. It will not be constructed/saved by the file system.

We'll be doing some work around this to normalize the name and hash the file name

migbot · 2021-06-16T22:11:48Z

app/database/manager/index.ts

+      const defaultDatabase = await this.getDefaultDatabase();
+      if (defaultDatabase) {
+          // retrieve recentlyViewedServers from Global entity
+          const recentlyViewedServers = await defaultDatabase.collections.get(GLOBAL).query(Q.where('name', RECENTLY_VIEWED_SERVERS)).fetch() as IGlobal[];


Will the change to use lastActiveAt be made in a separate PR?

migbot · 2021-06-16T22:13:40Z

app/database/manager/index.ts

@@ -428,4 +517,4 @@ if (!__DEV__) {
    logger.silence();
 }

-export default new DatabaseManager();
+export default DatabaseManager;


Why is this change necessary?

So as to use the DatabaseManager as a normal class rather than a Singleton.

we'll export this as a singleton.

migbot · 2021-06-16T22:17:04Z

app/database/operator/index.ts

-const DataOperator = new Operator();
+    constructor(database: Database) {
+        super();
+        this.activeDatabase = database;


Could we call this class DatabaseOperator? And also, since we'll have an instance of this class for each database, can we rename this.activeDatabase to this.database?

Could we call this class DatabaseOperator?

The purpose of this class is to treat data getting inside our database. It was initially called DataOperator and then we agreed to use the term Operator. I would prefer if we leave it as such. @enahum your thoughts on this ?

And also, since we'll have an instance of this class for each database, can we rename this.activeDatabase to this.database?

We are not using Singletons anymore. The Operator treats/manages data getting inside a server database. It is the Operator that relies on the DatabaseManager...not the other way around.

An instance of an operator needs an instance of a database connection for the activeDatabase but every Operator instances will point to the same Default database.

migbot · 2021-06-16T22:18:31Z

app/database/operator/utils/test.ts

@@ -71,7 +71,8 @@ describe('DataOperator: Utils tests', () => {
    it('=> sanitizeReactions: should triage between reactions that needs creation/deletion and emojis to be created', async () => {
        const dbName = 'server_schema_connection';
        const serverUrl = 'https://appv2.mattermost.com';
-        const database = await DatabaseManager.createDatabaseConnection({
+        const databaseManagerClient = new DatabaseManager();


Nit: I don't think we should append Client to these variables.

* MM_35115: Starting LoginOptions SSO * MM_36205: SSO [IN PROGRESS] * MM_36205 : SSO [ IN PROGRESS ] * Update sso_with_redirect_url.tsx * MM_36205 : SSO Tests [ IN PROGRESS ] * MM_36205 : Passing serverUrl to SSO screen * Update sso.test.tsx * Fix ViewTypes imports and keyMirror method * MM_36205 : Code clean up * Fix : Clean up imports * Update: Aligning with PR 5452 * Fix: AndroidManifest file to include redirection ofr scheme mmauthbeta * refactor: SSO Login method via Gitlab now navigates to Channel screen * refactor: SSO Login without redirectURL is also working * feat: SSO - main test completed * feat: ADDED test for sso_with_redirect_url * fix : eslint correction * fix: Updated Loading component name * fix : code clean up from reviews * fix: reviews check * fix: Added mmauthbeta into info.plist * Revert "fix: Added mmauthbeta into info.plist" This reverts commit d87cc23. * Update Info.plist * Update AppDelegate.m * feat: ADDED Forgot Password - Test [ IN PROGRESS ] * feat: Forgot Password - Completed & Tested * fix: Including MFA screen [ IN PROGRESS ] * MFA - Properly tested * Properly testing forgot_password screen * Fix login.test.tsx * Fix SSO method calls chain * Update index.tsx * Sort imports for sceen/navigation * fix: Reviews * Update signing + act in test * Removed todo comment on MFA * feedback review * fix login tests Co-authored-by: Avinash Lingaloo <> Co-authored-by: Elias Nahum <nahumhbl@gmail.com>

* fix: initial init refactor * fix: await isServerPresent * fix: more refactor * fix: move out launch functions * fix: remove comment * fix: update credential functions * fix: refactor launch functions * fix: deep link parsing * fix: lint change * fix: update deeplink and notification handlers * fix: indentation * fix: add relaunchApp * fix eslint * refactor launchProps and autoconnect server for deeplink * fix: use undefined * fix: define OptionalLaunchProps * fix: Android - handle server URL in push notification * fix: rename func * fix: use boolean launchError instead * fix: use DatabaseModule * fix: use DatabaseHelper instead * fix: remove unnecessary null check * fix: iOS - support for serverUrl * fix: iOS - extract serverUrl in reply action * fix: iOS - expose objc specific func * fix: remove unnecessary deviceToken param * fix: return if device is untrusted Co-authored-by: Elias Nahum <nahumhbl@gmail.com> * fix: bye bye modulePaths * fix: ios build * chore: remove unused aliases from babel.config * chore: fix dependency format in package-lock.json * chore: remove transparent window background color for android AppTheme * chore: remove mattermost.js and use index.ts as app entry * fix: login flow screens theme * fix: Launch types * chore: remove OptionalLaunchProps type * fix: url utils unit tests * chore: update en.json Co-authored-by: Elias Nahum <nahumhbl@gmail.com>

enahum · 2021-06-18T03:02:36Z

types/database/database.d.ts

+  serverUrl: string;
+};
+
+export type RetrievedDatabase = {


for when we go through the naming task this should be something like

export type ServerDatabase = Database & { displayName: string; url: string; }

enahum

yay!! let's merge this thing and go over all the comments for the next set of PR's

enahum · 2021-06-18T03:07:13Z

app/database/manager/index.ts

+          connection = serverDatabase.dbInstance;
+      } else {
+          // Or, it might be that the user has this server on the web-app but not mobile-app; so we'll need to create a new entry for this new serverUrl
+          const databaseName = connectionName ?? urlParse(serverUrl).hostname;


We'll be doing some work around this to normalize the name and hash the file name

enahum · 2021-06-18T03:11:26Z

app/database/manager/index.ts

+          // Or, it might be that the user has this server on the web-app but not mobile-app; so we'll need to create a new entry for this new serverUrl
+          const databaseName = connectionName ?? urlParse(serverUrl).hostname;
+          connection = await this.createDatabaseConnection({
+              shouldAddToDefaultDatabase: true,


should we separate concerns here? have the function just create the database and then we have another function to add the record into the default database?

If this function only applies to server databases then maybe we just rename the function to createServerDatabase

enahum · 2021-06-18T03:13:48Z

app/database/manager/index.ts

+          connection = await this.createDatabaseConnection({
+              shouldAddToDefaultDatabase: true,
+              configs: {
+                  actionsEnabled: true,


this should be applied only in the wdb call

enahum · 2021-06-18T03:19:48Z

app/database/manager/index.ts

+      }
+
+      if (setAsActiveDatabase) {
+          await this.setActiveServerDatabase(serverUrl);


we don't really need to await for this and setActiveServerDatabase all that is should do is update the default database server table to set the last_active_at field.

enahum · 2021-06-18T03:23:00Z

app/database/manager/index.ts

-          dbType = DatabaseType.DEFAULT,
-          serverUrl = undefined,
-      } = configs;
+      const {actionsEnabled = true, dbName = DEFAULT_DATABASE, dbType = DatabaseType.DEFAULT, serverUrl = undefined} = configs;


actionsEnabled does not need to be an argument

enahum · 2021-06-18T04:34:14Z

app/requests/remote/systems.ts

+    try {
+        data = await Client4.getDataRetentionPolicy();
+    } catch (error) {
+        forceLogoutIfNecessary(error);


for network client.. we need to handle this differently.. @migbot FYI

enahum · 2021-06-18T04:44:03Z

app/screens/server/index.tsx

+    // so that they are eventually accessible in the Channel screen.
+
+    const intl = useIntl();
+    const managedConfig = useManagedConfig<ManagedConfig>();


for the screen components, this could just be using the prop for managedConfig instead of using the hook. Both of the solutions would work just fine

enahum · 2021-06-18T04:45:40Z

app/utils/database.ts

+  displayName?: string;
+};
+
+export const createAndSetActiveDatabase = async ({serverUrl, displayName}: SetActiveDatabaseArgs) => {


do we need any of this? I believe this is already achievable with the DBManager Instance

enahum · 2021-06-18T04:46:11Z

app/utils/font_family.ts

    const oldRender = Text.render;
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore


let's use @ts-expect-error instead

enahum · 2021-06-18T04:48:43Z

app/utils/time_tracker.ts

@@ -2,7 +2,7 @@
 // See LICENSE.txt for license information.

 export default {


we can remove this file

migbot

Pair-reviewed with Elias.

Avinash Lingaloo and others added 30 commits April 29, 2021 18:33

MM_35115: ADDED select_server screen

de71f46

MM_35115: ADDED select_server screen

b08c731

MM_35115: ADDED files on which select_server is dependent

cfb33ee

MM_35115: ADDED react-native-button

85bdc06

MM_35115: Fixing TS issues [IN PROGRESS]

862d920

MM_35115: Started withObservables [IN PROGRESS]

683bd5b

MM_35115: Started withObservables [IN PROGRESS]

e8701f7

MM_35115: withObservables - defaulting when no connection is availabl…

9c30cad

…e [IN PROGRESS]

MM_35115: withObservables - some code clean up [IN PROGRESS]

3e4a896

MM_35115: withObservables - some code clean up [IN PROGRESS]

6da3475

MM_35115: withObservables - some code clean up [IN PROGRESS]

e9a12de

MM_35115: Substituting mapDispatchToProps [IN PROGRESS]

91e54f1

MM_35115: Substituting mapDispatchToProps [IN PROGRESS]

b9afded

MM_35115: Substituting mapDispatchToProps [IN PROGRESS]

36a8f95

MM_35115: Removed resetPing action [IN PROGRESS]

05078f4

MM_35115: ADDED app/client

2fc3941

MM_35115: Preparing scheduleExpiredNotification

1c28a22

MM_35115: Adding some todos

11c183b

Server & LoginOptions

22fec72

Use default server if available and autoconnect if configured

e92a8e9

Fix login header & manual server url

446202f

MM_35115: Login Options[IN PROGRESS]

f3917e6

MM_35115: Login screen - email [IN PROGRESS]

31ac1e3

MM_35115: Login screen - email [IN PROGRESS]

d05b3f2

MM_35115: Login screen - email - login api call [IN PROGRESS]

a5e8a77

MM_35115: Login screen - email - login api call [IN PROGRESS]

eca7411

MM_35115: Login screen - email - saving to server db [IN PROGRESS]

e77bc81

MM_35115: Login screen - email - saving to System, Preferences to db …

aeaf00c

…[IN PROGRESS]

MM_35115: Login screen - enforcing unique check on System entity [IN …

176c932

…PROGRESS]

MM_35115: Login screen - writing TeamMembership [IN PROGRESS]

d1605a5

Avinash Lingaloo and others added 14 commits June 8, 2021 15:16

MM_35115 : Refactoring set/getActiveDatabase to use flag record

06c4710

MM_35115 : Refactored active database to use flag in Global entity

65cf372

MM_35115 : Updated manual database manager test

92d405b

MM_35115 : Fix operator/utils/test

16e77a4

MM_35115 : Fix for base_handler

557d73d

MM_35115 : Fix test issues with Handlers

d37d05b

MM_35115 : Fix test issues with prepareRecords

a3982d8

MM_35115 : Fix wrapper test issue

64a28a6

MM_35115 : Updated getMostRecentServerConnection to return the server…

8680c8f

…Url as well as the connection

MM_35115 : Refactored the way we call DataOperator

80218f2

MM_35115 : Updated database manager mock

160dae0

Add getMostRecentServerUrl function (#5440)

48c7eae

* fix: add getMostRecentServerUrl func * fix: add ts and tsx to editorconfig * fix: rename functions * fix: return type

Fix unit test setup

ff8a09c

fix login screen unit tests

008a253

avinashlng1080 added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jun 14, 2021

migbot reviewed Jun 16, 2021

View reviewed changes

mm-cloud-bot added release-note-none and removed do-not-merge/release-note-label-needed labels Jun 18, 2021

enahum and others added 2 commits June 17, 2021 21:29

Merge branch 'gekidou' into MM_35115_gekidou_login_flow

cf96891

enahum reviewed Jun 18, 2021

View reviewed changes

enahum approved these changes Jun 18, 2021

View reviewed changes

enahum removed 2: Dev Review Requires review by a core commiter Work In Progress Not yet ready for review labels Jun 18, 2021

migbot self-requested a review June 18, 2021 04:57

migbot approved these changes Jun 18, 2021

View reviewed changes

enahum merged commit 3ee6e67 into gekidou Jun 18, 2021

enahum deleted the MM_35115_gekidou_login_flow branch June 18, 2021 04:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-35115 [Gekidou] Login flow - Email and Password #5402

MM-35115 [Gekidou] Login flow - Email and Password #5402

avinashlng1080 commented May 24, 2021 •

edited by enahum

Loading

migbot Jun 16, 2021

avinashlng1080 Jun 17, 2021

migbot Jun 16, 2021

migbot Jun 16, 2021

avinashlng1080 Jun 17, 2021

enahum Jun 18, 2021

migbot Jun 16, 2021

avinashlng1080 Jun 17, 2021

migbot Jun 16, 2021

avinashlng1080 Jun 17, 2021

enahum Jun 18, 2021

migbot Jun 16, 2021

avinashlng1080 Jun 17, 2021

migbot Jun 16, 2021

enahum Jun 18, 2021

enahum left a comment

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

enahum Jun 18, 2021

migbot left a comment

		@@ -2,7 +2,7 @@
		// See LICENSE.txt for license information.

		export default {

MM-35115 [Gekidou] Login flow - Email and Password #5402

MM-35115 [Gekidou] Login flow - Email and Password #5402

Conversation

avinashlng1080 commented May 24, 2021 • edited by enahum Loading

Summary

Ticket Link

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enahum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

migbot left a comment

Choose a reason for hiding this comment

avinashlng1080 commented May 24, 2021 •

edited by enahum

Loading