Graph QL POC #6024

larkox · 2022-03-03T17:11:53Z

Summary

This is just a draft to show off the progress made. Most of what has been done is just exploratory. It still can use lots of refinement.

Ticket Link

None

Release Note

NONE

agnivade · 2022-04-07T09:34:27Z

@larkox - Is this ready to be reviewed? I'd just like to take a look at the query.

larkox · 2022-04-07T10:06:27Z

@agnivade As POC, it is ready to review (the only "missing part" is the channel stats that were merged not long ago, but there won't be much difference).

My next objective (still not started) is to make this code "production ready", so any feedback on the current state is welcomed.

agnivade

Just a small thing that I spotted.

app/client/graphQL/entry.ts

agnivade

Ignore my previous comments. Queries look good to me. 👍

larkox · 2022-06-15T14:49:23Z

app/actions/remote/entry/common.ts

@@ -324,37 +326,12 @@ export const syncOtherServers = async (serverUrl: string) => {
        for (const server of servers) {
            if (server.url !== serverUrl && server.lastActiveAt > 0) {
                registerDeviceToken(server.url);
-                syncAllChannelMembers(server.url);
+                appEntry(server.url, server.lastActiveAt, false);


@enahum This is the design choice that probably I should revert. My idea here was to reuse all the logic behind the entry to sync the other servers, but I understand we already are going through the App entry when we switch, right?
So... we only need the channel members (no categories, no channels, no nothing) to calculate unreads and mentions. Am I right?

If so, I will revert this change and update the "syncAllChannelMembers" to use graphQL too.

Yeah we only need channel members and threads to show the mention badges. As we switch servers we go through the appEntry

app/client/graphQL/entry.ts

agnivade

Wohoo!

enahum

overall looks very good

app/actions/remote/entry/common.ts

app/actions/remote/entry/gql_common.ts

enahum · 2022-07-28T12:17:30Z

app/actions/remote/entry/gql_common.ts

+    if (prefData.preferences) {
+        const crtToggled = await getHasCRTChanged(database, prefData.preferences);
+        if (crtToggled) {
+            const {error} = await truncateCrtRelatedTables(serverUrl);


FYI @koox00

app/client/graphQL/types.ts

enahum · 2022-07-28T17:48:27Z

app/constants/custom_status.ts

-export enum CustomStatusDuration {
-    DONT_CLEAR = '',
-    THIRTY_MINUTES = 'thirty_minutes',
-    ONE_HOUR = 'one_hour',
-    FOUR_HOURS = 'four_hours',
-    TODAY = 'today',
-    THIS_WEEK = 'this_week',
-    DATE_AND_TIME = 'date_and_time',
-}


this is causing the tests to fail

enahum · 2022-07-28T17:48:40Z

app/constants/index.ts

@@ -40,7 +39,6 @@ export {
    Categories,
    Channel,
    Config,
-    CustomStatusDuration,


tests failing

enahum · 2022-07-28T17:48:59Z

app/screens/custom_status/components/clear_after.tsx

@@ -8,7 +8,7 @@ import {Text, TouchableOpacity, View} from 'react-native';
 import CompassIcon from '@components/compass_icon';
 import CustomStatusExpiry from '@components/custom_status/custom_status_expiry';
 import FormattedText from '@components/formatted_text';
-import {CustomStatusDuration, CST} from '@constants/custom_status';
+import {CST} from '@constants/custom_status';


test failing..

enahum · 2022-07-28T17:49:57Z

types/api/users.d.ts

+enum CustomStatusDuration {
+    DONT_CLEAR = '',
+    THIRTY_MINUTES = 'thirty_minutes',
+    ONE_HOUR = 'one_hour',
+    FOUR_HOURS = 'four_hours',
+    TODAY = 'today',
+    THIS_WEEK = 'this_week',
+    DATE_AND_TIME = 'date_and_time',
+}


I think we'll need a const for this as the tests are failing

enahum · 2022-07-29T11:35:29Z

@avinashlng1080 please review

avinashlng1080

LGTM. Just left a few comments that if need be can be tackled in future PRs

avinashlng1080 · 2022-07-29T11:43:08Z

app/screens/custom_status/components/clear_after.tsx

-        if (duration && duration === CustomStatusDuration.DATE_AND_TIME) {
+        if (duration && duration === 'date_and_time') {


Why not do duration === CustomStatusDurationEnum.DATE_AND_TIME ?

avinashlng1080 · 2022-07-29T11:52:42Z

app/screens/custom_status/components/custom_status_suggestions.tsx

-    {emoji: 'house', message: t('custom_status.suggestions.working_from_home'), messageDefault: 'Working from home', durationDefault: CustomStatusDuration.TODAY},
-    {emoji: 'palm_tree', message: t('custom_status.suggestions.on_a_vacation'), messageDefault: 'On a vacation', durationDefault: CustomStatusDuration.THIS_WEEK},
+    {emoji: 'calendar', message: t('custom_status.suggestions.in_a_meeting'), messageDefault: 'In a meeting', durationDefault: 'one_hour'},
+    {emoji: 'hamburger', message: t('custom_status.suggestions.out_for_lunch'), messageDefault: 'Out for lunch', durationDefault: 'thirty_minutes'},


Could we have done CustomStatusDurationEnum.THIRTY_MINUTES ?

We could, but I feel this is more idiomatic on how TypeScript works. The only one we use the enum in many places is the one that translates to '', since it is not readable enough.

avinashlng1080 · 2022-07-29T12:16:28Z

app/actions/remote/entry/gql_common.ts

+
+    if (chData?.channels?.length && chData.memberships?.length) {
+        // defer fetching posts for unread channels on initial team
+        fetchPostsForUnreadChannels(serverUrl, chData.channels, chData.memberships, initialChannelId);


On the Gekidou branch, we had a fix from PR #6453 to delay the fetchPostsForUnreadChannels call. Though it was a fix, we should check if this is happening on the 'GraphQL' flow as well.

First approach

649ec88

mm-cloud-bot added the release-note-none label Mar 3, 2022

larkox added 4 commits March 3, 2022 18:24

Lint

a0a2c74

Fixes and adding monitoring console statements (to be removed later)

6a344bc

Merge branch 'gekidou' into GraphQLPOC

f4ccede

Add pagination and apply graphQL also to login

80f6891

agnivade reviewed Apr 7, 2022

View reviewed changes

app/client/graphQL/entry.ts Outdated Show resolved Hide resolved

app/client/graphQL/entry.ts Outdated Show resolved Hide resolved

app/client/graphQL/entry.ts Outdated Show resolved Hide resolved

agnivade approved these changes Apr 19, 2022

View reviewed changes

larkox added 9 commits April 26, 2022 17:52

Merge branch 'gekidou' into GraphQLPOC

532352a

Get all entry points to use the same GQL call

cf0500d

Merge branch 'gekidou' into GraphQLPOC

0381be4

Unify gql handling

654c997

Use graphQL on websocket reconnect

d56500d

Merge branch 'gekidou' into GraphQLPOC

fe5516a

Merge branch 'gekidou' into GraphQLPOC

f9e7225

Merge branch 'gekidou' into GraphQLPOC

7f32c17

Handle latest changes regarding categories

451540e

larkox commented Jun 15, 2022

View reviewed changes

larkox added 4 commits June 17, 2022 18:58

Use graphQL to properly fetch channel members on other servers

7c0073f

Merge branch 'gekidou' into GraphQLPOC

c43946b

Remove logs and fetch unreads from other teams

7af977a

Minor fixes

5276861

larkox marked this pull request as ready for review July 6, 2022 11:46

larkox requested review from enahum and avinashlng1080 July 6, 2022 11:46

larkox added the 2: Dev Review Requires review by a core commiter label Jul 6, 2022

zefhemel reviewed Jul 7, 2022

View reviewed changes

app/client/graphQL/entry.ts Outdated Show resolved Hide resolved

Merge branch 'gekidou' into GraphQLPOC

8d54f0b

Final fixes

5a4da21

larkox requested a review from agnivade July 13, 2022 17:19

agnivade approved these changes Jul 13, 2022

View reviewed changes

zefhemel removed the request for review from enahum July 13, 2022 18:14

zefhemel approved these changes Jul 13, 2022

View reviewed changes

enahum requested changes Jul 28, 2022

View reviewed changes

larkox added 2 commits July 28, 2022 18:35

Merge branch 'gekidou' into GraphQLPOC

d63ecc4

Address feedback, minor refactoring, and fixes around the refactor

5c6f04a

enahum requested changes Jul 28, 2022

View reviewed changes

Fix custom status duration types

e6e512c

larkox requested a review from enahum July 28, 2022 19:46

Add missing fields and some reordering

6052c84

enahum approved these changes Jul 29, 2022

View reviewed changes

avinashlng1080 approved these changes Jul 29, 2022

View reviewed changes

avinashlng1080 added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Jul 29, 2022

Add timeout to fetch posts for unread channels

4149abc

larkox merged commit bae5477 into mattermost:gekidou Jul 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Graph QL POC #6024

Graph QL POC #6024

larkox commented Mar 3, 2022

agnivade commented Apr 7, 2022

larkox commented Apr 7, 2022

agnivade left a comment

agnivade left a comment

larkox Jun 15, 2022

enahum Jun 15, 2022

agnivade left a comment

enahum left a comment

enahum Jul 28, 2022

enahum Jul 28, 2022

enahum Jul 28, 2022

enahum Jul 28, 2022

enahum Jul 28, 2022

enahum commented Jul 29, 2022

avinashlng1080 left a comment

avinashlng1080 Jul 29, 2022

avinashlng1080 Jul 29, 2022

larkox Jul 29, 2022

avinashlng1080 Jul 29, 2022

		if (duration && duration === CustomStatusDuration.DATE_AND_TIME) {
		if (duration && duration === 'date_and_time') {

Graph QL POC #6024

Graph QL POC #6024

Conversation

larkox commented Mar 3, 2022

Summary

Ticket Link

Release Note

agnivade commented Apr 7, 2022

larkox commented Apr 7, 2022

agnivade left a comment

Choose a reason for hiding this comment

agnivade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agnivade left a 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

enahum commented Jul 29, 2022

avinashlng1080 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