search

bright-org / bright

3 stars 0 forks source link

PJルール策定、ツール導入検討など #21

Open Yoosuke opened 1 year ago

Yoosuke commented 1 year ago

０．本ルールの利用サイクル

１．判断基準

２．開発進捗管理／確認

３．顧客調整

４．ドキュメントの扱い方

５．内部会議体

リーダーMTG(毎週 月12:00～13:00)

ランチMTG（毎週 火12:00～13:00）

６．インフラ／開発環境

７．Github運用／CI

PhNの開発ブランチの運用

develop -> staging -> masterブランチを運用する(従来運用に同じ)

PhN開発中の保守ブランチの運用

hotfix及びmaintenanceブランチを運用する。

  1. master ブランチから、Dependabotによる更新を含め保守での対応を集約するブランチ（ maintenance ）を予め切っておく
  2. .github/dependabot.yml を作り、 maintenance ブランチにマージされるPRを出すように設定する
  3. PhXのリリースを追い越してリリースする場合、そのままmaintenanceブランチ上で各環境テストを実施する。(Stg環境は顧客およびPhNメンバーとの利用調整が必要)
  4. テストが終わったら、 maintenance ブランチを master ブランチにマージして本番リリースする。(保守案件のリリース)
  5. maintenance ブランチに変更が溜まったら、PRを作成し、PhNメンバーにマージ&テストを依頼する。

開発時ルール

初心者向けトピック

レビュー

８．リリース

９．UI設計／開発

9.1 画面開発流れ

  1. Wire / Design 資料を確認 (Figma)
  2. Markup (外注、skip)
  3. 基本の CRUD を開発 (phx.gen.live)
  4. 他の動作を開発

9.1.1 画面開発のPR分け方

9.2 UI 開発の必要な資料

9.3 assets/node_modules のインストール方法

docker compose run --rm web mix assets.setup # assets.setup は mix.exs で定義

9.4 phx.gen.live 使い方

Table を作成しない

mix phx.gen.live PresetTemplateCategories PresetTemplateCategory preset_template_categories name:string deleted_at:naive_datetime

Table 作成ずみ（API 開発時作成した）

mix phx.gen.live Histories History histories --no-context --no-schema

１０．API設計／開発

１１．DB設計／開発

１２．利用バージョン

・Elixir 1.12.x
・Phoenix 1.6.x
・LiveView 0.16.x
・Erlang/OTP 24.x
・Alpine.js 3.x
・tailwind components
  - Node.js XX.XX
  - npm XX.XX
・Github Actions
・GCP（GKE＋Cloud SQL）
・Sentry 8.x

１３．実装時ルール（コードレビュー観点／ポイントも）

■UI 　→UIページ名およびphx.gen.liveに従う 　　→コンテキスト 　　　→phx.gen.live Posts Post posts 　　　　phx.gen.live Writers Writer writers 　　　※既存のUI向け関数が、「api」配下にいる…

■Util 　→lib/dm_cluster/utils 　　→切り出したい人は切り出して 　　→モジュール名は名詞で組むこと（動詞や動詞＋erとしない） 　　→長いモジュール名は避けること 　　→カヲスったら、mokichi ＆ TJペアがどうにかする（レビュー段階で）


- Sentry へログを送信する場合

Sentry.capture_message/2 を使うこと


- [[⑬_1 エラーハンドリングのグランドルール]]

- [[⑬_2 ログのグランドルール]]

- [[⑬_3 Ecto DSLのグランドルール]]

- [[⑬_4 テストのグランドルール]]

- [[⑬_5 UI開発のグランドルール]]

# １４．このWikiの更新について
* ルールはslack等で議論してから追加すること
* 初心者（見習い）向けトピックは自由に記述してもよい、但し初心者（見習い）向けとわかるように明記すること
* Wikiを更新後は全体で共有すること
[12:21](https://digi-dock.slack.com/archives/D039KUABW3S/p1685157661251469)
⑥_1 障害監視インフラの構成
# 監視者
- DMクラスター社の監視者が監視を行う
  - DMクラスター社で契約したSentryを利用
  - DMクラスター社側の監視者のGmail宛に送付される

# 監視インフラ構成
`アプリケーションログ -> Sentry -> Gmail`

の流れで、異常系ログを送信することで監視を行う。

## アプリケーションログ -> Sentry
アプリケーションから [Sentry.capture_message/2](https://hexdocs.pm/sentry/Sentry.html#capture_message/2)  をコールすることで `アプリケーション -> Sentry` にログを送る。

## Sentry -> Gmail
Sentry のメール通知機能を用いて `Sentry -> Gmail` 通知を行う。

# Sentry
[Sentry]([https://sentry.io/welcome/)というエラー監視をはじめとする様々なトラッキング機能を提供する](https://sentry.io/welcome/)%E3%81%A8%E3%81%84%E3%81%86%E3%82%A8%E3%83%A9%E3%83%BC%E7%9B%A3%E8%A6%96%E3%82%92%E3%81%AF%E3%81%98%E3%82%81%E3%81%A8%E3%81%99%E3%82%8B%E6%A7%98%E3%80%85%E3%81%AA%E3%83%88%E3%83%A9%E3%83%83%E3%82%AD%E3%83%B3%E3%82%B0%E6%A9%9F%E8%83%BD%E3%82%92%E6%8F%90%E4%BE%9B%E3%81%99%E3%82%8B) SaaS を採用。

## 料金プランと制限
無料の Developer プランを使用。
1アカウント / 月5000ログ制限 / ログの保持期間30日 などの制限がある。

- https://sentry.io/pricing/

## エラーの外部サービスへの通知機能について
無料の Developer プランだと Gmail 通知は可能だが、 Sentry の Integration 機能を使って Slack などに通知するのは不可能（Teamプラン以上でなければならない）。

現状の要件では Gmail 通知までできればいいので問題ないが、Slack などの外部サービスへの通知を行いたい場合は Zapier や GAS などで自前で作りこむ、もしくは Team プラン以上にアップグレードして Integration 機能を使うことを検討。

## アプリケーションが使用するSentryライブラリ
アプリケーションではこちらのSentry公式のライブラリを使用

- https://hexdocs.pm/sentry/Sentry.html

## フィジビリティ確認
Phoenix アプリケーションで Sentry を導入し Gmail にメール通知するまでのフィジビリティ確認を行い、以下の記事にまとめた。

- https://qiita.com/koyo-miyamura/items/52cd7732e745281b30f7

# 参考資料
- 2021/09 開発開始時点でのログ設計
  - https://github.com/karabiner-inc/dm_cluster/issues/79
- Sentry 公式のElxiirライブラリ
  - https://hexdocs.pm/sentry/Sentry.html
- Sentry が公式が提供しているElixirでの導入方法
  - https://docs.sentry.io/platforms/elixir/
[12:21](https://digi-dock.slack.com/archives/D039KUABW3S/p1685157677048739)
⑬_1 エラーハンドリングのグランドルール
# API／UI共通

エラーのおおまかな考え方は以下 

- 準正常系とするもの
  - バリデーションエラー
  - メイン処理は成功し後処理でエラーになるようなもの
  - 基本的にはSentryへの通知は必要ない(`Logger.error` は使わず `Logger.warning` を使う)
- 異常系とするもの
  - システムエラー
  - 後続の処理を中断し、ユーザーに500を返す/エラーページを表示する必要があるもの
  - 基本的にはSentryへの通知はする(ただし`Logger.error` は使わず例外を投げる想定)

## 準正常系の実装方針

- 準正常系のエラーを返す可能性がある`Context`, `Util` は以下の形式で値を返す

  ```elixir
  {:ok, any()} | {:error, error_type :: atom(), params :: any()}

異常系の実装方針

基本的には異常系のエラーが起こった場合は例外をraiseする　or !付きの関数を使う エラーハンドリングは DmClusterWeb.ErrorHandlerで行う

異常系のテストについて

Context

API

準正常系の実装方針（4xx系）

基本的にFallbackControllerで処理する

異常系の実装方針（5xx系）

UI

（TJさん、tamanugiさん抜き出し後に差異があれば埋め合わせお願いします）

HTTPステータスコード毎のエラーページ

ステータスコード エラーページ要否とページ内容の整理
400 Bad Request エラー通りで良い
→タイトル「リクエストが正しくありません」
→メッセージ「リクエストが正しく無いため、アクセスできない状態です[br]お手数ですが、URLを見直してください」
→「/」に戻るボタン付き
401 Unauthorized ログインページにリダイレクトするのでエラーページ不要
402 Payment Required 該当機能無し
403 Forbidden 404として扱う
★404 Not Found エラー通りで良い　※基本400と同じデザイン
→タイトル「ページが見つかりません」
→メッセージ「大変申し訳ありません[br]お探しのページは見つかりませんでした」
→「/」に戻るボタン付き
405 Method Not Allowed エラー詳細を隠したいので400エラーと同じで良い
406 Not Acceptable エラー詳細を隠したいので400エラーと同じで良い
400系の以降 エラー詳細を隠したいので400エラーと同じで良い
★500系全般 エラー詳細を隠したいので単一メッセージで良い　※基本400と同じデザイン
→タイトル「アクセスできない状態です」
→メッセージ「サーバーにアクセスが集中しているか、メンテナンスを実施中のため、一時的にアクセスできない状態です[br]申し訳ありませんが、少し時間を空けてから再度アクセスを行ってください」
→「/」に戻るボタン付き

準正常系の実装方針（4xx系） ※正常系メッセージ表示も包含する

各フォーム入力時エラーは、各入力項目の直下にリアルタイム表示（LiveView通り）

    <div class="mb-6">
      <%= label f, :name, "企業名", class: "text-xs mb-0.5 block" %>
      <%= text_input f, :name, class: "input" %>
      <%= error_tag f, :name %>
    </div>

異常系の実装方針（5xx系）

Yoosuke commented 1 year ago

⑥_1 障害監視インフラの構成

監視者

監視インフラ構成

アプリケーションログ -> Sentry -> Gmail

の流れで、異常系ログを送信することで監視を行う。

アプリケーションログ -> Sentry

アプリケーションから Sentry.capture_message/2 をコールすることで アプリケーション -> Sentry にログを送る。

Sentry -> Gmail

Sentry のメール通知機能を用いて Sentry -> Gmail 通知を行う。

Sentry

Sentryというエラー監視をはじめとする様々なトラッキング機能を提供する](https://sentry.io/welcome/)%E3%81%A8%E3%81%84%E3%81%86%E3%82%A8%E3%83%A9%E3%83%BC%E7%9B%A3%E8%A6%96%E3%82%92%E3%81%AF%E3%81%98%E3%82%81%E3%81%A8%E3%81%99%E3%82%8B%E6%A7%98%E3%80%85%E3%81%AA%E3%83%88%E3%83%A9%E3%83%83%E3%82%AD%E3%83%B3%E3%82%B0%E6%A9%9F%E8%83%BD%E3%82%92%E6%8F%90%E4%BE%9B%E3%81%99%E3%82%8B) SaaS を採用。

料金プランと制限

無料の Developer プランを使用。 1アカウント / 月5000ログ制限 / ログの保持期間30日 などの制限がある。

エラーの外部サービスへの通知機能について

無料の Developer プランだと Gmail 通知は可能だが、 Sentry の Integration 機能を使って Slack などに通知するのは不可能（Teamプラン以上でなければならない）。

現状の要件では Gmail 通知までできればいいので問題ないが、Slack などの外部サービスへの通知を行いたい場合は Zapier や GAS などで自前で作りこむ、もしくは Team プラン以上にアップグレードして Integration 機能を使うことを検討。

アプリケーションが使用するSentryライブラリ

アプリケーションではこちらのSentry公式のライブラリを使用

フィジビリティ確認

Phoenix アプリケーションで Sentry を導入し Gmail にメール通知するまでのフィジビリティ確認を行い、以下の記事にまとめた。

参考資料

Yoosuke commented 1 year ago

⑬_1 エラーハンドリングのグランドルール

API／UI共通

エラーのおおまかな考え方は以下

準正常系の実装方針

異常系の実装方針

基本的には異常系のエラーが起こった場合は例外をraiseする　or !付きの関数を使う エラーハンドリングは DmClusterWeb.ErrorHandlerで行う

異常系のテストについて

Context

API

準正常系の実装方針（4xx系）

基本的にFallbackControllerで処理する

異常系の実装方針（5xx系）

UI

（TJさん、tamanugiさん抜き出し後に差異があれば埋め合わせお願いします）

HTTPステータスコード毎のエラーページ

ステータスコード エラーページ要否とページ内容の整理
400 Bad Request エラー通りで良い
→タイトル「リクエストが正しくありません」
→メッセージ「リクエストが正しく無いため、アクセスできない状態です[br]お手数ですが、URLを見直してください」
→「/」に戻るボタン付き
401 Unauthorized ログインページにリダイレクトするのでエラーページ不要
402 Payment Required 該当機能無し
403 Forbidden 404として扱う
★404 Not Found エラー通りで良い　※基本400と同じデザイン
→タイトル「ページが見つかりません」
→メッセージ「大変申し訳ありません[br]お探しのページは見つかりませんでした」
→「/」に戻るボタン付き
405 Method Not Allowed エラー詳細を隠したいので400エラーと同じで良い
406 Not Acceptable エラー詳細を隠したいので400エラーと同じで良い
400系の以降 エラー詳細を隠したいので400エラーと同じで良い
★500系全般 エラー詳細を隠したいので単一メッセージで良い　※基本400と同じデザイン
→タイトル「アクセスできない状態です」
→メッセージ「サーバーにアクセスが集中しているか、メンテナンスを実施中のため、一時的にアクセスできない状態です[br]申し訳ありませんが、少し時間を空けてから再度アクセスを行ってください」
→「/」に戻るボタン付き

準正常系の実装方針（4xx系） ※正常系メッセージ表示も包含する

各フォーム入力時エラーは、各入力項目の直下にリアルタイム表示（LiveView通り）

    <div class="mb-6">
      <%= label f, :name, "企業名", class: "text-xs mb-0.5 block" %>
      <%= text_input f, :name, class: "input" %>
      <%= error_tag f, :name %>
    </div>

異常系の実装方針（5xx系）

Yoosuke commented 1 year ago

⑬_2 ログのグランドルール

グランドルール

ログメッセージについて

ログレベルについて

ログレベルの基準は以下のとおり

Cloud Loggingでのログの確認方法

  1. GCPコンソールの ロギング - ログエクスプローラーを選択
  2. ログクエリに以下の値を入力する
resource.type="k8s_container"
resource.labels.project_id="dm-cluster-dm-send-dev"

※環境ごとに値は異なる

Yoosuke commented 1 year ago

⑬_3 Ecto DSLのグランドルール

1.ルール

2.バッドパターン例

駄目な例:fromを使っていない

def get_history_from_order_number!(order_number) do
    History
    |> where([h], h.order_number == ^order_number)
    |> Repo.one!()
end

改善例

from(history in History,
  where: history.order_number == ^order_number
)
|> Repo.one!

駄目な例:fromでないかつjoinの引数が増えてよろしくない

参考元 https://medium.com/flatiron-labs/how-to-compose-queries-in-ecto-b71311729dac

  def with_transmission(query \\ Car, type) do
    query
    |> join(:left, [c], s in Specification, as: :specifications, on: c.specification_id == s.id)
    |> join(:left, [c, specifications: s], t in Transmission, as: :transmissions, on: s.transmission_id == t.id)
    |> where([c, transmissions: t], t.type == ^type)
  end

駄目な例:fromでないかつjoinの引数を省略すると変数が暗黙になるため事故を起こしやすい

 def with_transmission(query \\ Car, type) do
    query
    |> join(:left, [c], s in Specification, as: :specifications, on: c.specification_id == s.id)
    |> join(:left,  t in Transmission, as: :transmissions, on: s.transmission_id == t.id)
    |> where( t.type == ^type)
  end

join時のon:の駄目な例: 複数の連結条件はandを使用しないSQLと互換性がない書き方

from(history in History,
      join: item in Item,
      on: [id: history.item_id, company_id: history.company_id]
    )

改善例

from(history in History,
      join: item in Item,
      on: history.item_id == item.id and history.company_id == item.company_id
    )

（ymnさんリード）

Yoosuke commented 1 year ago

⑬_4 テストのグランドルール

前提：正常系や異常系といった用語の認識合わせ

種別 説明 備考
正常系 ある操作がエラーを出さずに処理されることをテストする HTTPにおけるステータスコード 2xx や 3xx のイメージ
準正常系 ある操作が想定済みのエラー（必須項目が入力されていない等）を出すことをテストする HTTPにおけるステータスコード 4xx のイメージ
異常系 入力やアプリ自体に問題がなくても出てしまうエラー（DBが落ちている等）がハンドリングされることをテストする HTTPにおけるステータスコード 5xx のイメージ

詳しくは以下のページを参照

https://wa3.i-3-i.info/diff610test.html

モックの利用シーン

状況依存を再現するためモックを使うが、本プロジェクトでは2つのライブラリを利用する

ライブラリ 利用シーン ドキュメント
mock モジュールの関数のうちいくつかをモック化できればいい場合 https://hexdocs.pm/mock/Mock.html
mox モジュールを丸ごとモック化したい場合 https://hexdocs.pm/mox/Mox.html

mockライブラリを使う上での注意点

モック化する範囲は最小限に抑えること

例：何らかの原因でDBへの書き込みが失敗するケース

DmCluster.Histories.update_history_status/1 は内部で複数の処理を行うが、DBへの書き込みが失敗することをシミュレーションするには

といった具合に、モック化する範囲が最小限になるようにする

DB（PostgreSQL）

Phoenixには最初からテスト用DBを用意してテストを実行できる環境が整っているため、基本的にはPhoenixの作法に従う

正常系

テスト用DBに実際にデータを入れたり参照したりする

準正常系

テスト用DBに実際にデータを入れたり参照したりする

異常系

mock を使って Postgrex.Error が発生することをシミュレーションする（テスト用DBをダウンさせたりはしない）

実装は以下を参考

https://github.com/karabiner-inc/dm_cluster/blob/9e82114cc3cf27699cfc2ead36bf0bca54ce2b2a/server/test/dm_cluster/histories_test.exs#L162-L175

GCPのCloud Storage

正常系/準正常系/異常系すべて、 mox を使ってファイル操作をシミュレーションする（GCPにはアクセスしない）

実装は以下を参考（Cloud Storageを使っている箇所のテストではないが、moxの使い方という点では参考になるはず）

https://github.com/karabiner-inc/dm_cluster/blob/9e82114cc3cf27699cfc2ead36bf0bca54ce2b2a/server/test/dm_cluster/direct_mails_test.exs#L20-L26

SCPホットフォルダ

正常系/準正常系/異常系すべて、 mox を使ってファイル操作をシミュレーションする（SSHやSCPは行わない）

実装は以下を参考

https://github.com/karabiner-inc/dm_cluster/blob/9e82114cc3cf27699cfc2ead36bf0bca54ce2b2a/server/test/dm_cluster/direct_mails_test.exs#L20-L26

メール送信

DmCluster.Mailer.deliver/1 でメール送信処理を行うが、 Swoosh.Adapters.Test を使うため、実際にメールが送信されることはない

正常系

DmCluster.Mailer.deliver/1 でメール送信処理を行う

準正常系

DmCluster.Mailer.deliver/1 でメール送信処理を行う

異常系

mock を使って Swoosh.DeliveryError が発生することをシミュレーションする？

※詳しい調査が必要だが、エラーを投げなくても {:error, ...} を返せばいいだけ かもしれない

Sentry

TODO

Yoosuke commented 1 year ago

⑬_5 UI開発のグランドルール

dmクラスターの共通仕様

作成,更新,削除

バリデーション

UI バリデーションメッセージ（項目毎など）のデザインが未確定

更新とサブミット両方 <.form let={f} for={@changeset} phx-change="validate" phx-submit="save">

削除の挙動

作成,更新,削除 後の挙動

ポップアップ

モーダル

ページネーション

UI ページネーションの動作の統一化

page と page_size が query param に定義されて、例：?page=1&page_size=10

フォーム

注意点がある要素のみ

DatePicker

フラッシュメッセージ

検索

検索条件の種類は以下

キーワード検索(企業名、アイテム名など)

-- アイテム名: アイテム で検索した場合
SELECT
...
WHERE
name LIKE '%アイテム%';

日付検索 (登録日付、 更新日付など)

-- 検索条件が 登録日付 2022/01/30 ~ 2022/02/03 の場合

SELECT
...
WHERE
inserted_at >= '2022/01/29 15:00:00'
AND
inserted_at <= '2022/02/03 14:59:59'

選択検索 (カテゴリ, ステータスなど)

検索条件のリセット

検索後に別ページに遷移したあとに戻った場合の検索条件

検索動作

ページネーション後に検索が行われる場合、検索後は1ページ目に戻る（= 検索後は常に1ページ目に戻る）

UI 検索リセット動作の統一化

:index にリダイレクト、例：<%= live_redirect "リセット" to: Routes.item_approval_index_path(DmClusterWeb.Endpoint, :index) %>

UI 検索時はURLパラメータを残すか？

はい、URLパラメータを残す。

UI 正常系メッセージの出力タイミング

動作完了後。

例：

{:ok, _item_approvals} ->
        {:noreply,
         socket
         |> put_flash(:info, "Item approvals updated successfully")
         |> push_redirect(to: socket.assigns.return_to)}

ページネーションと検索の組み合わせ時動作

エラーハンドリング ★要確認

システムエラー

→ 500 ページが表示される

業務エラー

→ フラッシュメッセージが表示される

UIの場合のエラーログメッセージはフォーマットしない

長い文字列の文字溢れについて

UIで使う文字フォーマット共通関数

例

・送付状況　DmCluster.Formatters.HistoryFormatter

・日付　DmCluster.Formatters.DateTimeFormatter

共通関数実装例（共通関数側） https://github.com/karabiner-inc/dm_cluster/pull/322

共通関数適用例（画面側） https://github.com/karabiner-inc/dm_cluster/pull/333/files

一覧ソート

更新日時 desc

order by updated_at desc;

また、updated_atが、PostgreSQLだと秒単位になり、順不同になるリスクがあるため、次点のorder byとしてid（降順、desc）を入れること。

Alpine.js と LiveView の使い分け

方針：Server と連携が必要な部分は LiveView にする、UI（表示系）だけな部分は Alpine.js を利用する。

具体的な例：

Alpline.js Tips

alpine.js で @click イベントで select inputを実装した場合、併せて @click.outside イベントも指定し、フォーカスが外れたときに閉じるようにする

Yoosuke commented 1 year ago
Yoosuke commented 1 year ago

確認案

０．本ルールの利用サイクル フェーズ切り替え時には、本ルールを必ず全員で見返すこと フェーズ切り替え時に限らず、疑問を持ったときや、問い合わせ先が分からないときも、本ルールを必ず参照すること

１．判断基準 どんどんメンバー間でコミュニケーション取ってもらって、交流深めたり、連携してくださいませ(｀･ω･´)ゞ 話の宛先や判断に困るときや、本ページに書かれていないことは、@Yoosuke、もしくは @piacere に相談してください 顧客折衝やお金周り、あと既存環境とかでも無い限り、@piacere に許可を取る必要は無く、各役割やメンバーとのコミュニケーションを取ることで解決を試みてください 自信が無い場合や、心配ある場合は、事前に @Yoosuke に共有いただくとグッドです 議論の場に、@Yoosuke, @piacerex を巻き込んでいただくのも、お気軽にどうぞ @Yoosukeは毎朝 08:00 ~ 08:30はFace to Face で相談できる時間としておきます。

定例で使っているZoomを、定例以外の個別メンバー間でご利用いただくのもOKです 利用時間帯がぶつかったときは、よしなに調整してください…あまりにぶつかるときは、別途追加発行もします

意見が割れて、収束がつきにくいとき等は、最終的に @piacere がジャッジを行うようにしますので、呼び出してください ＆ ご安心ください(^o^)

言いにくいことがあっても、言わないと何も生まれません … 言って良い場なので、意見を出しましょう

Yoosuke commented 1 year ago

基本設計レビュールール確定したら一旦クローズ