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

[DOJO-46] ログイン機能・ページを実装 #33

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

iusok3386
Copy link
Collaborator

@iusok3386 iusok3386 commented Feb 8, 2025

詳細の記入が遅れてすみません。完了いたしました。

Jira

DOJO-46

やったこと

  • Strapi と Github OAuth を使ったログイン機能を追加
    • 順序
      1. フロントエンド /login ページにアクセスし、ログインボタンを押下
      2. /login -> Strapi -> GitHub OAuth へリダイレクト
      3. GitHub OAuth で各個人のアカウントにログイン & 認証
      4. GitHub OAuth -> Strapi へリダイレクト
        • この時には最終的なアクセストークンではなく、一時的な認証用コードがクエリパラメータとして渡される
      5. Strapi -> GitHub OAuth へ再リダイレクト
        • 一時的な認証用コードをクエリパラメータとして渡す
      6. GitHub OAuth -> Strapi へリダイレクト
        • 最終的なアクセストークンがクエリパラメータとして渡される
      7. Strapi -> フロントエンド /
        • 最終的なアクセストークンがクエリパラメータとして渡される
        • アクセストークンが Cookie に保存される
    • Cookie のスコープは環境変数 HOST (サーバーのドメイン) で定義
  • アカウントメニューを試作
    • 現在のところ、ユーザー名・メールアドレス・ログアウトボタンを表示
  • src フォルダへの移行に関わる不具合の修正
    [DOJO-45] フォルダ構成の整理 (修正1) #35 で修正します
    • shadcn の設定を修正 (components.json)
      • pnpm dlx shadcn@latest add [xxx] でコンポーネントを追加できないことがあったのが修正された
    • ESLint の設定を修正 (eslint.config.mjs)
      • src/*.{tsx,ts} のファイルに対して全く動作していなかった

要検討事項

  • 環境変数の命名
    • Strapi の環境変数で、フロントエンド側と競合しそうなものは STRAPI_[XXX] に変更している
  • フロント側はログインしないとページを閲覧できなくなるので、README.md に GitHub OAuth のセットアップ方法を追記すべき
  • フロント側で各ページにアクセスするたびに Strapi にユーザー情報を問い合わせることになっている。
    以下の 2 つのアクセスが毎回発生する
    • src/middleware.ts: サイトを遷移する前に、ログインしているかどうかを確認
    • src/components/my-account.tsx: ユーザー情報 (名前・メールアドレス) を取得

必要な設定

  1. docker compose up strapi で Strapi を起動してください。初期設定が完了していない場合は、それも行ってください。
  2. Strapi のガイド の「Set Up Github Provider in Strapi Admin Dashboard」に従って、GitHub OAuth をご自身ののアカウントで設定し、Strapi の設定も完了させてください。

動作風景

@iusok3386 iusok3386 self-assigned this Feb 8, 2025
@@ -2,8 +2,8 @@ import { Button } from '@/components/ui/button';
import { ReactNode } from 'react';

interface PrimaryButtonProps {
onClick: (e: React.FormEvent) => void;
children: ReactNode;
onClick?: (e: React.FormEvent) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

childrenがnullableなのは分かりますが、onClickが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.

言われてみれば。
Header を SSR にしたくて強引に仕様を変更していたところがあるので、もう一度方法を検討してみます。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正してみました
ただ、実際に実行や提出を行うフローにどのように繋げるか、
まだ見当がついていません。
7da07f2

@@ -5,13 +5,13 @@ ARG NODE_ENV=development
ENV NODE_ENV=${NODE_ENV}

WORKDIR /opt/
COPY package.json package-lock.json ./
COPY package.json ./
Copy link
Contributor

Choose a reason for hiding this comment

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

package-lock.json を再利用しないなら、.gitignoreに定義して、そもそもリポジトリに含まない、としても良さそうですね

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しました
8e3b6fc

host: env('HOST', '0.0.0.0'),
port: env.int('PORT', 1337),
host: env('STRAPI_HOST', '0.0.0.0'),
port: env.int('STRAPI_PORT', 1337),
Copy link
Contributor

Choose a reason for hiding this comment

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

小さなことですが、1337 にように、説明のない数字のことをマジックナンバーと言い、バグの原因になりやすいです。

例えば1337で言えば、3行目と7行目に使われています。誰かが3行目を変更しても7行目を変更し忘れたら、正しく動かなくなります。
(このような小さなスコープなら問題は起きにくいですが、複数ファイルに渡りマジックナンバーが使用されると、バグになりやすいです。
なので、定数を定義するファイルを作り、そこにexport const STRAPI_PORT = 1337 のように定義すると良いでしょう

Copy link
Collaborator Author

@iusok3386 iusok3386 Feb 11, 2025

Choose a reason for hiding this comment

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

マジックナンバー、こういった細かな気遣いがソフトウェアの持続性を向上させるのですね、、、

修正しました
79a0ff9

`/submissions/[id]/edit` 以外のページでヘッダーボダンが表示されないように変更
@iusok3386 iusok3386 requested a review from wf9a5m75 February 11, 2025 15:11
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.

2 participants