複数LMSのサポート #192

kou029w · 2021-02-03T11:39:23Z

close #180

!! NOTICE: 破壊的な変更が含まれます !!

feat: lti_consumer テーブルの追加
fix: 不要なmigrationsが行われる問題への対処 see also Migrate creates unnecessary migrations prisma/prisma#5244
feat: 複数のLMSのサポート - 環境変数からではなく lti_consumer テーブルから見つける
chore: bump prisma version
refactor: use include option in upsertTopic()
fix: seeding error:*.upsert()を同期的に実行すると誤って重複するので修正
refactor: Unknown→UnlinkedProblem,*NotFoundProblem - create Problem component
feat: global session atom
feat: NEXT_PUBLIC_LMS_URL をやめる - 代わりに LtiLaunchBody.launch_presentation_return_url を使う
chore: ビルドプロセスの整理
docs: LTI Tool Consumer 追加手順を記載

本番環境での移行方法

マイグレーション後、次のようなSQLを実行してください。 (@ties-makimura)

例:
OAuth Consumer key: {key}
OAuth Consumer secret: {secret}
の場合

-- Insert empty `lti_consumer`
INSERT INTO "lti_consumer" VALUES ('{key}', '{secret}');

-- Set `consumer_id` and `lti_consumer_id`
UPDATE "lti_context" SET consumer_id = '{key}';
UPDATE "lti_resource_link" SET consumer_id = '{key}';
UPDATE "users" SET lti_consumer_id = '{key}';

その他詳しい情報は README にも記載してます。
ご確認のほどよろしくお願いします。

確認したこと

ローカル環境でマイグレーションが機能することを確認。

git checkout feat-support-multiple-lms
docker-compose rm -sf db && docker-compose up -d db && sleep 5 && yarn --cwd server migrate

以前のバージョンをチェックアウトしマイグレーション・シーディングした後でも同様に機能することを確認。

git checkout master
docker-compose rm -sf db
docker-compose up -d db moodle
sleep 5
yarn --cwd server migrate
yarn --cwd server build:prisma
yarn --cwd server seed
git checkout feat-support-multiple-lms
yarn dev

いずれも問題無くパス。
LMSのリンク(とその更新)も機能している。

vercel · 2021-02-03T11:39:26Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/ties-makimura/chibi-ch-i-lo/oyitxvuyn
✅ Preview: https://chibi-ch-i-lo-git-feat-support-multiple-lms.ties-makimura.vercel.app

see also prisma/prisma#5244

- 環境変数からではなく `lti_consumer` テーブルから見つける

- create Problem component

- 代わりに LtiLaunchBody.launch_presentation_return_url を使う

knokmki612 · 2021-02-04T02:38:27Z

「確認したこと」に記載のあるコマンドを実行して、私のローカル環境でも正常にLTIリンクからちびチロにアクセスできることを確認しました！

knokmki612 · 2021-02-04T03:04:43Z

server/prisma/seed.ts

-  await Promise.all(
-    ltiResourceLinks
-      .map((link) => ({ ...link, bookId: createdBooks[0].id }))
-      .map(upsertLtiResourceLink)
-  );
+  for (const link of ltiResourceLinks) {
+    await upsertLtiResourceLink({
+      ...link,
+      consumerId: ltiConsumer.id,
+      bookId: createdBooks[0].id,
+    });
+  }


Promise.allを使ったほうが効率がよさそうに思ったのですが、
ba3b76c のコミットメッセージをみると、一連のPromiseにたいして非同期にupsertすると、insertで？重複するので、個別にawaitすることで解決した、という意味なのかなと思いました

私の手元の環境だけかもしれませんが、

await Promise.all(topics.map((topic) => upsertTopic(authorId, topic)));

初回実行時にこの部分で重複するIDのトピック1つ必ず作られており修正しました。
Prismaの実装を詳しく追ったわけでないので正確なことは分かりませんがおそらくON CONFLICT句等でアトミックにINSERTしているわけではなくSELECTクエリとINSERTクエリそれぞれ発行されて1つ前のINSERTの実行以前にSELECTが実行されるなどの要因で誤ったINSERTやUPDATEが発行されているのではないかという仮説があり、とりあえずfor文で直列に実行されるように対処して初回実行時にupsertたちが失敗しないように修正しました。(今読み返すとupsertUserの部分も修正しておくべきですね。。。)

prisma/prisma#3242 これかな?

knokmki612 · 2021-02-04T03:16:47Z

pages/book/topic/import/index.tsx

@@ -13,18 +13,16 @@ import { pagesPath } from "$utils/$path";
 export type Query = BookEditQuery;

 function Import({ bookId, context }: BookEditQuery) {
-  const { data: session } = useSession();
+  const { isTopicEditable } = useSessionAtom();


isTopicEditableとisBookEditableをuseSessionInitで用意してから使いまわせるようにしたんですね、スッキリしましたね！

knokmki612

良さそうに思いました！

kou029w · 2021-02-04T05:29:31Z

マージします。
検証環境、本番相当環境へのマイグレーションのご対応のほどをお願いします。 > @ties-makimura
振る舞いについてそのほか問題等あれば別途起票ください

kou029w requested review from ties-makimura and knokmki612 February 3, 2021 11:39

kou029w force-pushed the feat-support-multiple-lms branch from 4110956 to 9f809a1 Compare February 3, 2021 11:46

vercel bot deployed to Preview February 3, 2021 11:46 View deployment

kou029w added 8 commits February 3, 2021 20:56

feat: lti_consumer テーブルの追加

06f1de2

fix: 不要なmigrationsが行われる問題への対処

11b4713

see also prisma/prisma#5244

feat: 複数のLMSのサポート

a9aeb3e

- 環境変数からではなく `lti_consumer` テーブルから見つける

chore: bump prisma version

d153926

refactor: use include option in upsertTopic()

63ab7c2

fix: seeding error:*.upsert()を同期的に実行すると誤って重複するので修正

ba3b76c

refactor: Unknown→UnlinkedProblem,*NotFoundProblem

2268ae7

- create Problem component

feat: global session atom

47141ac

kou029w force-pushed the feat-support-multiple-lms branch from 9f809a1 to 95744a4 Compare February 3, 2021 11:58

vercel bot deployed to Preview February 3, 2021 11:58 View deployment

kou029w added 3 commits February 3, 2021 21:00

feat: NEXT_PUBLIC_LMS_URL をやめる

b75e914

- 代わりに LtiLaunchBody.launch_presentation_return_url を使う

chore: ビルドプロセスの整理

c1dde31

docs: LTI Tool Consumer 追加手順を記載

b963a5e

kou029w force-pushed the feat-support-multiple-lms branch from 95744a4 to b963a5e Compare February 3, 2021 12:00

vercel bot deployed to Preview February 3, 2021 12:02 View deployment

docs: Prisma Studio を使った設定方法を記載

197fd5f

vercel bot deployed to Preview February 4, 2021 02:09 View deployment

kou029w added this to 現在進行中 in CHiBi-CHiLO via automation Feb 4, 2021

knokmki612 reviewed Feb 4, 2021

View reviewed changes

knokmki612 approved these changes Feb 4, 2021

View reviewed changes

fix: seed 初回実行時誤って重複する恐れがある問題の対処 (upsertUser())

11ed01b

vercel bot deployed to Preview February 4, 2021 04:04 View deployment

docs: seed関連TODO

ab021b2

vercel bot deployed to Preview February 4, 2021 04:22 View deployment

kou029w merged commit 9a97ec1 into master Feb 4, 2021

CHiBi-CHiLO automation moved this from 現在進行中 to Done Feb 4, 2021

kou029w deleted the feat-support-multiple-lms branch February 4, 2021 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

複数LMSのサポート #192

複数LMSのサポート #192

kou029w commented Feb 3, 2021 •

edited

vercel bot commented Feb 3, 2021 •

edited

knokmki612 commented Feb 4, 2021

knokmki612 Feb 4, 2021

kou029w Feb 4, 2021 •

edited

kou029w Feb 4, 2021 •

edited

knokmki612 Feb 4, 2021

knokmki612 left a comment

kou029w commented Feb 4, 2021 •

edited

複数LMSのサポート #192

複数LMSのサポート #192

Conversation

kou029w commented Feb 3, 2021 • edited

本番環境での移行方法

確認したこと

vercel bot commented Feb 3, 2021 • edited

knokmki612 commented Feb 4, 2021

knokmki612 Feb 4, 2021

Choose a reason for hiding this comment

kou029w Feb 4, 2021 • edited

Choose a reason for hiding this comment

kou029w Feb 4, 2021 • edited

Choose a reason for hiding this comment

knokmki612 Feb 4, 2021

Choose a reason for hiding this comment

knokmki612 left a comment

Choose a reason for hiding this comment

kou029w commented Feb 4, 2021 • edited

kou029w commented Feb 3, 2021 •

edited

vercel bot commented Feb 3, 2021 •

edited

kou029w Feb 4, 2021 •

edited

kou029w Feb 4, 2021 •

edited

kou029w commented Feb 4, 2021 •

edited