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

予算管理ページのapi定義 #900

Merged
merged 8 commits into from
Dec 30, 2024

Conversation

Kubosaka
Copy link
Collaborator

@Kubosaka Kubosaka commented Dec 26, 2024

対応Issue

resolve #901

概要

  • 予算管理ページで必要なAPIをopenapiで定義した
  • /financial_records
    • 支出元 局など
  • /divisions
    • 部門
  • /festival_items
    • 物品

画面スクリーンショット等

  • URL

テスト項目

  • apiの定義が正しいか確認する

備考

apiは実装していないです。

figma
ER図

@Kubosaka Kubosaka changed the title 予算管理ページのapi 予算管理ページのapi定義 Dec 26, 2024
@Kubosaka Kubosaka self-assigned this Dec 26, 2024
@Kubosaka Kubosaka requested a review from Chikuwa0141 December 26, 2024 13:43
@hikahana
Copy link
Collaborator

suggestion
swaggerで見たいからgenerateまでしてもいいかも

リンクだけじゃなくて該当箇所のスクショもあったら凄く嬉しいです!!
私も次からそうします!

Copy link
Collaborator

@hikahana hikahana left a comment

Choose a reason for hiding this comment

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

簡単にコードレビューしました!

application/json:
schema:
$ref: "#/components/schemas/division"
required: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
required: false
required: true

imo
更新だしtrueでも良さそう

Copy link
Collaborator

Choose a reason for hiding this comment

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

他も同様

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -767,6 +831,134 @@ paths:
"200":
description: yearで指定されたexpensesを取得
content: {}
/festival_items:
Copy link
Collaborator

Choose a reason for hiding this comment

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

PRはfinacial_itemsと書かれてましたがfestival_itemsが正しい方ですよね.。
正直どっちでもいいとは思いますけど。

responses:
"200":
description: 作成されたdivisionが返ってくる
content: {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
content: {}
content:
application/json:
schema:
$ref: "#/components/schemas/division"

type: objectでもよいかも?

imo
仮初めだけどcontentは書きたいマン

Copy link
Collaborator

Choose a reason for hiding this comment

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

他も同様

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も含めて返したいと思い、getで配列で返しているオブジェクトを返すようにしました
response修正

Comment on lines +2072 to +2090
festivalItem:
required:
- name
- divisionId
- amount
type: object
properties:
name:
type: string
example: 農ポリ
divisionId:
type: integer
example: 1
amount:
type: integer
example: 10000
memo:
type: string
example: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

question
festtival_itemとitem_budgetが統合されてる感じがしますけどがpostとputの際どうする予定とかあります?
普通にテーブル統合してpostするだけ?
image

Copy link
Collaborator Author

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.

なら良さそう!

@@ -1927,6 +2255,18 @@ components:
remark:
type: string
example: ""
total:
Copy link
Collaborator

Choose a reason for hiding this comment

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

このレビューに関係ないので読み飛ばし可です。

これ別個でAPI生やして管理します?それとも各APIに組み込ますか?
totalの使い方まだピンと来てないマンです。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

api内で集計して、totalに整形して返すイメージです。
局、部門、物品で合計が必要になるので、レスポンスで返す共通の型を定義した感じです。
apiを生やす?ってのがピンときていないです

Copy link
Collaborator

Choose a reason for hiding this comment

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

totalって何回も算出されそうだからtotal用のAPI作ってもいいのかなって思ってだったんですけど、よく考えたらべつにAPI作る必要なさそうですね

@hikahana
Copy link
Collaborator

hikahana commented Dec 26, 2024

あと私のgo.mod以下みたいになったんですけどなります?
websocketが消えた

module github.com/NUTFes/FinanSu/api

go 1.16

require (
	github.com/go-sql-driver/mysql v1.6.0
	github.com/go-test/deep v1.0.8 // indirect
	github.com/google/go-cmp v0.6.0 // indirect
	github.com/joho/godotenv v1.4.0
	github.com/labstack/echo/v4 v4.11.4
	github.com/oapi-codegen/runtime v1.1.1
	github.com/pkg/errors v0.9.1
	github.com/slack-go/slack v0.13.0
	github.com/stretchr/testify v1.9.0 // indirect
	golang.org/x/crypto v0.23.0
	golang.org/x/net v0.25.0 // indirect
	gorm.io/driver/mysql v1.3.3
	gorm.io/gorm v1.23.4
)

Base automatically changed from feat/kubosaka/fix-openapi to develop December 27, 2024 01:14
@Kubosaka
Copy link
Collaborator Author

自分も消えました
使ってないようなので消した方でコミットしました
#900 (comment)

Copy link
Collaborator

@hikahana hikahana left a comment

Choose a reason for hiding this comment

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

LGTM

@Kubosaka Kubosaka merged commit 997ccae into develop Dec 30, 2024
1 check passed
@Kubosaka Kubosaka deleted the feat/kubosaka/create-budgets-openapi branch December 30, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

予算管理のAPIを定義する
2 participants