Skip to content
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

DB を PostgreSQL に移行 #13

Merged
merged 42 commits into from
Dec 3, 2022
Merged

DB を PostgreSQL に移行 #13

merged 42 commits into from
Dec 3, 2022

Conversation

rokoucha
Copy link
Collaborator

@rokoucha rokoucha commented Nov 27, 2022

rokoucha and others added 6 commits November 27, 2022 23:57
Co-authored-by: rinsuki <428rinsuki+contact.github@gmail.com>
Co-authored-by: rinsuki <428rinsuki+contact.github@gmail.com>
Co-authored-by: tosuke <tosuke@users.noreply.github.com>
@SnO2WMaN
Copy link
Member

SnO2WMaN commented Nov 28, 2022

タグのデータ持ち方多分大幅に変えるので置き換えるのであればなるべく後の方に回してください(#15 で修正予定)

rokoucha and others added 3 commits November 28, 2022 22:48
Co-authored-by: rinsuki <428rinsuki+contact.github@gmail.com>
Co-authored-by: tosuke <tosuke@users.noreply.github.com>
Co-authored-by: rinsuki <428rinsuki+contact.github@gmail.com>
Co-authored-by: SnO₂WMaN <me@sno2wman.net>
@SnO2WMaN
Copy link
Member

SnO2WMaN commented Dec 1, 2022

手元で検証するとき面倒なのでAuthorizationヘッダにトークン入れて認証するハッチ/graphqlに残しておいてください

@rokoucha
Copy link
Collaborator Author

rokoucha commented Dec 1, 2022

手元で検証するとき面倒なのでAuthorizationヘッダにトークン入れて認証するハッチ/graphqlに残しておいてください

3268515 いれた

@SnO2WMaN SnO2WMaN marked this pull request as ready for review December 2, 2022 01:33
src/index.ts Outdated
.object({
query: z.string(),
variables: z.optional(z.record(z.string(), z.any())),
operationName: z.optional(z.string()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operationNamez.optionalではなくz.nullable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なおす


function videoEntityToGraphQLVideo(video: Video) {
return {
id: "video:" + video.id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

返却される値にvideo:のprefixが付いているのはGraphQLの設計としておかしいと思う(取得時の引数には必要がないので)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rinsuki これなんでつけたんだっけ、自分も昨日見て思い出せなかった

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

その他同様

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GraphQLのidって基本的にそれだけで取れないといけないんじゃないっけ (node(id: ID)に渡されるので)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あー GraphQL API の空間内でユニークな ID じゃないと駄目ってことか

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

じゃあ base64 とか通す?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

良いけど,長すぎない?

echo "video:01GK78BVF1CQGPW58FKSABW65G" | base64
dmlkZW86MDFHSzc4QlZGMUNRR1BXNThGS1NBQlc2NUcK

Copy link
Member

@SnO2WMaN SnO2WMaN Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

別にweb/クライアント側で取得時に適当にprefix付け外したりbase64エンコードなりすれば良いか

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かにGraphQL上のidとは別にrecordIdとかあってもいいかも

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

それはちょっと煩雑感あるねー良い感じにグローバルでウニークな ID 採番できると良いんだけど

@rokoucha rokoucha changed the title Chore/postgres DB を PostgreSQL に移行 Dec 3, 2022
@rokoucha rokoucha marked this pull request as ready for review December 3, 2022 12:26
@SnO2WMaN
Copy link
Member

SnO2WMaN commented Dec 3, 2022

registerVideoのmutationで以下.

mutation RegisterVideo($input: RegisterVideoInput!) {
  registerVideo(input: $input) {
    video {
      id
      title
      tags {
        id
        name
      }
    }
  }
}
{
  "input": {
    "primaryTitle": "カゲロウジジイズ",
    "primaryThumbnail":"http://nicovideo.cdn.nimg.jp/thumbnails/40926580/40926580.84316399",
    "tags": [],
    "sources": [{"type": "NICOVIDEO",  "sourceId":"sm40926580" }]
  }
}

備考: "sources": []で動作.

{
  "errors": [
    {
      "message": "null value in column \"source\" of relation \"video_sources\" violates not-null constraint",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "registerVideo"
      ]
    }
  ],
  "data": null
}

rokoucha and others added 16 commits December 3, 2022 22:10
Co-authored-by: SnO₂WMaN <me@sno2wman.net>
Co-authored-by: rinsuki <428rinsuki+contact.github@gmail.com>
Co-authored-by: SnO₂WMaN <me@sno2wman.net>
Co-authored-by: rinsuki <428rinsuki+contact.github@gmail.com>
Co-authored-by: tosuke <tosuke@users.noreply.github.com>
Co-authored-by: Rokoucha <git@rokoucha.net>
Co-authored-by: SnO₂WMaN <me@sno2wman.net>
Co-authored-by: rinsuki <428rinsuki+git@gmail.com>
Co-authored-by: SnO2WMaN <me@sno2wman.net>
@rinsuki rinsuki merged commit 2eddb7b into main Dec 3, 2022
@rinsuki rinsuki deleted the chore/postgres branch December 3, 2022 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants