hyochan / hackatalk-server Goto Github PK

Followed by #30, we need to add [Reaction] model and resolver.

@ daadaadaah

@Jeontaeyun

#40 #41

Development environment faced the error

OS : macOS Catalina 10.15.3
MySQL : 8.0.19
Node : v12.16.1
NPM : 6.13.4
yarn : v1.22.4
Sequelize CLI : 5.5.1
ORM: 5.21.5

Expected behavior

Actual behavior

Steps to reproduce the behavior

1. Clone hackatalk-server repo

2. yarn install

3. Create test_db Database

4. Create MySQL user (ID : hackatalk, PW: hackatalk!)

5. Give user(ID: hackatalk) permission

6. yarn migrate:dev

7. yarn start

8. Access http://localhost:4000/graphql

9. Execute Signup Mutation three times

10. Execute createChannel Mutation

11. The following error occurred. This error occurred when I executed galleries Query, createGallery Mutation.
    

12. The same error occurred when I accessed https://stage.hackatalk.dev/graphql
    

cf. The follow image is when I executed createGallery Mutation and galleries Query

1. createGallery Query
   

2. galleries Query
   

Seeing the mobile code, I found there's something missing props from server.
Seeing the capture below, there's photos in each user's information area.

so thumbURL and photoURL in input type called UserQueryInput needed.

When users update profile, their profile photos should be updated to photoURL and resized photo to thumbURL.

here's UserQueryInput input props now.

should I PR about to resolve that? or something else needed to check?

please let me know.

User resolver should provide like statement in query and pagination

It should have unit tests for each GraphQL resolvers of the following.

• chat
• chatroom
• friend
• user

It should have unit tests for sequelize queries of the following.

• Auth
• Chat
• Chatroom
• Friend
• Notification
• Review
• User

We want to implement scalar File upload provided from apollo-server 2.0.

• Reference

Above can be used in our profileUpdate mutation and later createMessage mutation with file type.

Description

Situation

When you test relay-style pagination.

Problem

To implement 'users' query below, we need 'cursor' value.
In this example, it is before: "1584861818000"

query {
    users (
        last: 1,
        before: "1584861818000",
    ) {
        totalCount
        edges {
            node {
                id
                name
                createdAt
            }
            cursor
        }
        pageInfo {
            endCursor
            hasNextPage
            startCursor
            hasPreviousPage
        }
    }
}

To get this value, we can use database tool such as DBeaver.

Or you can use mysql shell as below

To use the value as cursor for relay-style pagination, we need to convert it with javascript.
We picked the data with user in the third row that has name value 'geoseong'

$ new Date("2020-03-22 07:23:38.0").getTime()
1584829418000

Let's run 'users' query with before & last: 1 keyword

query {
    users (
        last: 1,
        before: "1584861818000",
    ) {
         ...
    }
}

The result is this.

{
    "data": {
        "users": {
            "edges": [
                {
                    "node": {
                        "id": "0c962590-6c0e-11ea-862d-69363194e26b",
                        "name": "geoseong",
                        "createdAt": "2020-03-22T07:23:38.000Z"
                    },
                    "cursor": "1584861818000"
                }
            ],
            "pageInfo": {
                "endCursor": "1584861818000",
                "hasNextPage": false,
                "startCursor": "1584861818000",
                "hasPreviousPage": true
            }
        }
    }
}

The result is the third user 'geoseong'. If the result is correct, the data returned should be the second user 'hyochan'.
This is because we use before keyword, the user 'geoseong' which used as a cursor should not be included in the result.

Cause

When we get the value from database tool or mysql shell, the data was like this.

2020-03-22 07:23:38

But, in fact, the more correct data is below.

"2020-03-22T07:23:38.000Z"

You can get this data with user query.

query {
  user(id: "0c962590-6c0e-11ea-862d-69363194e26b") {
    id
    email
    createdAt
  }
}

{
    "data": {
        "user": {
            "id": "0c962590-6c0e-11ea-862d-69363194e26b",
            "email": "[email protected]",
            "createdAt": "2020-03-22T07:23:38.000Z"
        }
    }
}

This produces a different result from the previous data.

$ new Date("2020-03-22T07:23:38.000Z").getTime()
1584861818000

Let's run query again.

query {
    users (
        last: 1,
        before: "1584861818000",
    ) {
         ...
    }
}

Now we get a correct user 'hyochan'

{
    "data": {
        "users": {
            "totalCount": 1,
            "edges": [
                {
                    "node": {
                        "id": "0fb4f1c0-6c0e-11ea-862d-69363194e26b",
                        "name": "hyochan",
                        "createdAt": "2020-03-22T07:23:43.000Z"
                    },
                    "cursor": "1584861823000"
                }
            ],
            "pageInfo": {
                "endCursor": "1584861823000",
                "hasNextPage": false,
                "startCursor": "1584861823000",
                "hasPreviousPage": true
            }
        }
    }
}

Result

When you need to test a query, you need to call user query first.
Then get the createdAt data to use as a 'cursor' value.

[Problem]
We've previously implemented createMessage resolver in #63. However, this was initial proposal that relies on puree2e encryption supported by virgil sdk.

We faced below problems relying on the current features.

1. Most critically, users who've joined a channel can't leave the channel at will.
   • This happened because we've tried to follow the Direct Messages in slack.
2. When users are removed from the created channels, there will be the same member group and it makes more complexity on separating channels in this case. We don't want to make things complicated.

Therefore, I thought it'd be good to resolve the above issue and also support the group chat as default. 1 on 1 basis chat would also rely on the group chat (this means in virgil sdk).

Since then, we have to always createChannel before making messages.

I need to use friends query to solve that issue(hackatalk-mobile #106).

However, while looking up the list of friends, I found a problem.

src/resolvers/friend.ts line 18

const friendIds = await friendModel.findAll({
    attributes: ['userId'], // <-- Expected with ['friendId'].
    where: { userId: { [Op.eq]: auth.id } },
    raw: true,
});

const friendArray = [];
friendIds.forEach((friendId) => {
    friendArray.push(friendId.userId);
});

const friends = await userModel.findAll({
    where: { id: { [Op.in]: friendArray } },
    raw: true,
});

It's not accurate to figure out the db structure, but this code is expected to return my information, not a list of friends.

If my opinion is correct, I hope someone fixes it quickly.

If I need to fix it, I would do something like this:

const firendIds = await friendModel.findAll({
    attributes: ['friendId'],
    where: { userId: { [Op.eq]: auth.id } },
    raw: true,
});

const friends = await userModel.findAll({
    where: {
        id: { [Op.in]: firendIds.map(({ friendId }) => friendId) },
    },
    raw: true,
});

Followed by the client integration issue

Listing up todos

** Client dependent **

• Manage on / offline users
• Email verification mutation
• Add gallery
• Change password
• Add channel
• Send message
• Reply to message
• Add reaction to message
• Get messages with pagination
  • example
  • client
• Channel subscription
• Message subscription

** Server side only **

• Utilities
  • e2e encryption
  • Check valid fb id
    • reference
  • Check valid Google token
    • reference
  • Check valid Apple id
    • Apple signin tutorial

#10

Recently, gallery and reaction models are added. Sync this to sequelize models and graphql schemas.

• Migration for tests as shared in gallery resolver - @dooboolab
• Support relay style pagination as described in link - @geoseong
  • Reference
  • Continue Mar 8th.
• Implement singleUpload resolver by - @YongPilMoon
  • Related #33
  • Continue Mar 8th.
• Implement Reaction model & schema & resolver - @daadaadaah
  • Emoji test in client
• Research Virgil Security on rotate key - @seonghyeonkimm
• Readme update - @godon019
• Sequelize migration medium - @smallbee3
  • Support for Reaction model.
• Support localization through the context - @bang9 @Jeontaeyun
• Service layer refactoring

( The explanation below is based on the commit 3fd3994, that has migration file. But with current commit, the migration file has been deleted.)

If you set up the local environment with "sequelize.sync()" command by executing "yarn local", you will face the error below.

$ yarn migrate:local

Sequelize CLI [Node: 10.18.0, CLI: 5.5.1, ORM: 5.21.4]

Loaded configuration file "config/config.js".
== 20200127105045-Channel: migrating =======

ERROR: Duplicate column name 'name'

The problem is that your local database is already synced with the models defined in 'src/models' through "sequelize.sync();" (src/db/index.ts)

Let me make an example. Suppose you are making the local environment.
After creating the database, you probably execute 'yarn start' or 'yarn local'.
(Because 'yarn migrate:local' at this point will give you an error, too)
Thanks to 'sequelize.sync()' function, you don't need to care about clumsy migration work.
However, because you haven't execute the command "sequelize db:migrate(yarn migrate:local)", all of your migration files are still pending.
It would be even worse if you have to make and run a new migration. Since 'yarn migrate' command run migration files from the first file, you only get an error.
The first migration file is this.

'20200127105045-Channel.js'

module.exports = {
  up: (queryInterface, Sequelize) => {
   return queryInterface.addColumn(
      'channels',
      'name',
      {
        type: Sequelize.STRING,
        after: "type",
      },
    );
  },

  down: (queryInterface, Sequelize) => {
    return queryInterface.removeColumn(
      'channels',
      'name',
    );
  }
};

Because what you ordered to sequeilze is that adding already existing columns 'channels' and 'name' to 'Channel' model, you will receive an error message as below.

"ERROR: Duplicate column name 'name'"

At this point, you might have to follow this process.(which is awful way)

1. Remove all previous migration files
2. Run your newly made migration file
3. Recover all deleted migration files at No.1

In conclusion, the solution seems that eradicate 'sync' option and only use 'migration files'.
(But, of course we can use 'sync' when we make test code.)

I made a pull request regarding the above issue.
I added the createTable migration files for every model. And also I removed 'sequelize.sync()' option but instead make 'sequelize db:migrate' command run each time you execute 'yarn start' or 'yarn local'.

(before)

...
"scripts": {
    "local": "cross-env NODE_ENV=local nodemon --exec ts-node src/server.ts",
    "start": "cross-env NODE_ENV=development nodemon --exec ts-node src/server.ts",
    ...

(after)

...
"scripts": {
    "local": "cross-env NODE_ENV=local sequelize db:migrate && cross-env NODE_ENV=local nodemon --exec ts-node src/server.ts",
    "start": "cross-env NODE_ENV=development sequelize db:migrate && cross-env NODE_ENV=development nodemon --exec ts-node src/server.ts",
    ...

Here are some references that I've read.

Best Paractice: DB Sync + Migrations? Or just Migrations?
sequelize/sequelize#4160

Sequelize Sync vs Migrations
https://stackoverflow.com/questions/41595755/sequelize-sync-vs-migrations

Sequelize.js: how to use migrations and sync
https://stackoverflow.com/questions/21105748/sequelize-js-how-to-use-migrations-and-sync

Any other opinions?

There are duplicate error handlings and error types in our codes.
Like to share the better ways to handle this.

Related #50

This issue follows after merging #13.

Sequelize model parameter should not pass as an argument for convenience.
The model objects for the methods to query, should be used directly in each model file.

For example,

Bad

// src/model/Chat.ts

export const getChatsByChatroomId = (Chat, chatroomId) => {
  ...
};

Good

// src/model/Chat.ts

class Chat extends Model {
  ...
}
Chat.init({
  ...
}
export const getChatsByChatroomId = (chatroomId) => {  // No model parameter
  ...
};

derived from dooboolab-community/hackatalk#98

Status

Goal

• Implementing Profile upload related APIs with test codes.
• Implementing File upload API with test codes. #33
• Implementing Profile upload related API calls in hackatalk-mobile.

Releated sources

• hacktalk-mobile - ProfileUpdate.tsx

Requirements

• Hackatalk mobile (react-native)

  • Resizing an image (in Hackatalk app)
    • expo - image manipulator
  • API call - Uploading an image to the server
    • by REST API - /upload_single
  • API call - Update profile
    • by GraphQL query

• Hackatalk server (express server)

  • API - Uploading an image to the server (Already implemented. Refactoring, if needed)
    • by REST API - /upload_single
  • API - Update profile (Already implemented. Refactoring, if needed)
    • by GraphQL Apollo server
    • GraphQL schemes
    • GraphQL resolvers
    • Sequelize models

We'd like to localize our graphql server response with i18n.