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

merge updates of optinist-for-server #67

Merged
merged 20 commits into from
Sep 26, 2023

Conversation

ReiHashimoto
Copy link

@ReiHashimoto ReiHashimoto commented Sep 22, 2023

主な取り込み内容

新規プロット(Pie Chart, Line Graph, Histogram, Polar Plot)の追加

マルチユーザー向け機能

その他動作改善

備考

  • マルチユーザーモードの切り替えについて
    • BE, FEそれぞれの環境変数を編集する必要がある
      • デフォルトはスタンドアロン
      • APIでauthなどのdependenciesの必要有無を判定するため、BEのenvにもIS_STANDALONEを追加している
  • DBについては現状docker-composeはなし
    • ローカルのDBを構築し、そちらに接続
    • sqliteはupdated_atなどのカラムに使用しているtextが互換性がないため、現状見送り

Next ToDo

  • マルチユーザー機能に関するドキュメンテーション
  • 権限種類の追加
    • optinist-for-server固有のrole定義だったため、現状adminのみ移管
    • admin以外のgeneralなロールを定義する
  • seedの対応
    • role、初期ユーザー、組織などを投入するスクリプトなどの準備

@ReiHashimoto ReiHashimoto self-assigned this Sep 22, 2023
@itutu-tienday
Copy link
Collaborator

itutu-tienday commented Sep 25, 2023

@ReiHashimoto
当バージョンの環境構築手順について、以下でOKでしょうか

  1. pip install -e '.[dev]'
  2. 環境情報設定
    • BE
      • config/*
    • FE
      • .env.*
  3. DB準備
    1. DB構築(MySQL互換)
    2. schme作成 alembic upgrade head
    3. DB初期データ投入 (users,role,あたりか)

@itutu-tienday
Copy link
Collaborator

itutu-tienday commented Sep 25, 2023

@ReiHashimoto
当PRについて、ざっと動作確認を実施。以下レポートを記載します

  • 確認内容

    • issueの機能リスト一通り動作確認
      • 一旦マルチユーザー版を確認
      • ※確認済みの項目には checklist に check
  • 確認結果

    • 各機能の基本的な動作は、OKである模様(仕様面、動作面(特に動作エラーなし))
    • 品質面の詳細確認については、詳細テストケースを起こして実施する形が良い
      • 合わせて画面設計書(ベース版)も起こしておくと良い
  • 指摘

@ReiHashimoto
Copy link
Author

@itutu-tienday

@ReiHashimoto 当バージョンの環境構築手順について、以下でOKでしょうか

  1. pip install -e '.[dev]'

  2. 環境情報設定

    • BE

      • config/*
    • FE

      • .env.*
  3. DB準備

    1. DB構築(MySQL互換)
    2. schme作成 alembic upgrade head
    3. DB初期データ投入 (users,role,あたりか)

リリース時にreadthedocsにsetup手順を掲載予定です。
以下の内容を想定しています。
https://drive.google.com/file/d/1MJIZGV6T81XWwogEaddMLJHC7p76tgiC/view?usp=drive_link

  • 開発作業がないため、pip install .でOKです。
  • あとはFirebaseプロジェクトの準備が必要なくらいで、ほぼ認識一致しているかと思います。

@ReiHashimoto
Copy link
Author

@itutu-tienday

認証
oist#130
以下の機能が取り込まれていない
arayabrain#98 (comment)
※そもそも、for server版でも動作していない?(デグレの可能性)

PRの内容としては以下の処理が該当するとは思われますが、確かにfor-server含め動作が正常ではないかもしれません

@ReiHashimoto
Copy link
Author

@itutu-tienday

環境設定
動作モードの切替(Standalone/MultiUser)には、FEとBEでそれぞれ環境変数の設定を必要としているが、設定不備があった場合、容易に検知できるように考慮は行われているか?
→ 構成の不備を検知しづらい状態とならないかを懸念

  • 現状こちらは良い手法が見つかっておらず、ToBeとさせていただきたいです。

Workflow
oist#144
これは現バージョンで動作確認可能でしょうか?

  • 対象のプロットを生成する関数が現状ないため、確認が難しいです。テストコード一式のアップデートの際に、ダミーで出力できる関数などの実装を検討しようと思います。

権限制御
oist#86
標準で一般権限(Operator)を用意しておくと良いか

  • 460e506 で、OPERATOR: 20を追加することで対応しました。ドキュメントもこれを踏まえて調整予定です。

@itutu-tienday
Copy link
Collaborator

@ReiHashimoto

直近のリリースでは対応外でOKと思いますが、以下の不具合となるため、また修正を想定いただければと思います。

  • 不具合) マルチユーザーモード時、ログインセッションがタイム・アウトしても、Dashboard画面へは遷移ができてしまう。

@itutu-tienday
Copy link
Collaborator

@ReiHashimoto

  • 現状こちらは良い手法が見つかっておらず、ToBeとさせていただきたいです。

了解しました。

  • 対象のプロットを生成する関数が現状ないため、確認が難しいです。テストコード一式のアップデートの際に、ダミーで出力できる関数などの実装を検討しようと思います。

現時点では確認対象外(直近に利用がない機能)で了解しました。

  • 460e506 で、OPERATOR: 20を追加することで対応しました。ドキュメントもこれを踏まえて調整予定です。

修正を確認しました。

@ReiHashimoto ReiHashimoto merged commit dc2dadc into develop-main Sep 26, 2023
3 checks passed
@ReiHashimoto ReiHashimoto deleted the feature/merge-for-server branch September 26, 2023 09:45
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