GetQuestionnaireMyRemindStatusとEditQuestionnaireMyRemindStatusの再実装 #1303
Conversation
…lete CancelTargets methods to ITarget interface
model/targets_impl.go
Outdated
| @@ -104,8 +104,33 @@ func (*Target) IsTargetingMe(ctx context.Context, questionnairID int, userID str | |||
| return false, nil | |||
| } | |||
|
|
|||
| // CancelTargets アンケートの対象をキャンセル(削除しない) | |||
| func (*Target) CancelTargets(ctx context.Context, questionnaireID int, targets []string) error { | |||
| func (*Target) GetTargetsRemindStatus(ctx context.Context, questionnaireID int, targets []string) ([]bool, error) { | |||
There was a problem hiding this comment.
ここの返り値の順序ってtargetsと必ず一致します?(一致するなら問題ないですが、しなかったら構造体配列のほうがよさそう)
There was a problem hiding this comment.
ご指摘ありがとうございます。試したところ確かに問題があるのを確認しました
単一ユーザーのみに使われているため、targets []stringからtarget []stringに変更する方針にしたいと考えています
|
これ、現時点では単一ユーザーにしか使ってないっぽいんですけど、他の人のリマインドステータスとりたいとかいう要望後から出ませんかね?(ここは一般化してしまっても良い気がしています) |
model/targets.go
Outdated
| @@ -10,6 +10,6 @@ type ITarget interface { | |||
| DeleteTargets(ctx context.Context, questionnaireID int) error | |||
| GetTargets(ctx context.Context, questionnaireIDs []int) ([]Targets, error) | |||
| IsTargetingMe(ctx context.Context, quesionnairID int, userID string) (bool, error) | |||
| GetTargetsRemindStatus(ctx context.Context, questionnaireID int, targets []string) ([]bool, error) | |||
| GetTargetsRemindStatus(ctx context.Context, questionnaireID int, target string) (bool, error) | |||
There was a problem hiding this comment.
単数になったので、このままいくなら関数名も単数にお願いします
リマインドをオン/オフにするかどうかは、あくまで個人の好みであり、他人に知られる必要のない情報であると思うので、他の人のステータスをまとめて取得する必要があまり出てこないのではないかと思います |
|
管理者が欲しかったりしないですかね?(答える気のある人がどのくらい答えてないのか) あと、今のリマインドってDMじゃなくてどっかのチャンネルにメンション流す仕組みのはずなのでメンション分析すればわかる機能な気がします |
|
全体の統計であればGetTargetsからすでに取得できるので、特定の複数ユーザのリマインダ情報を取得するのはあんまり多くないかな |
|
あ、UpdateTargetsRemindStatusはすでに複数ユーザ対応の実装だったですね... |
…tTargetsCancelStatus and UpdateTargetsCancelStatus, and update related logic
There was a problem hiding this comment.
Pull Request Overview
This PR reimplements the methods to get and update a user's reminder status for a questionnaire. The changes include new model methods (GetTargetsCancelStatus and UpdateTargetsCancelStatus) with corresponding tests, interface updates, and adjustments to handler and controller endpoints to include userID handling.
- Introduces new functions for retrieving and updating targets' cancellation status.
- Updates test cases in model/targets_test.go to verify different target scenarios.
- Adjusts handler and controller logic to pass the userID and correctly invert the cancellation flag.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| model/targets_test.go | Adds tests for new GetTargetsCancelStatus and UpdateTargetsCancelStatus behaviors. |
| model/targets_impl.go | Replaces CancelTargets with GetTargetsCancelStatus and UpdateTargetsCancelStatus implementations. |
| model/targets.go | Updates the ITarget interface to reflect the new methods. |
| handler/questionnaire.go | Adjusts the endpoints to retrieve userID and pass it to the model methods. |
| controller/questionnaire.go | Passes userID and updates status logic (inverting the cancellation flag) accordingly. |
Comments suppressed due to low confidence (3)
model/targets_impl.go:127
- [nitpick] Consider renaming the parameter 'cancelStatus' to 'remindStatus' to match the name in the interface in model/targets.go for consistency.
func (*Target) UpdateTargetsCancelStatus(ctx context.Context, questionnaireID int, targets []string, cancelStatus bool) error {
model/targets.go:14
- [nitpick] The parameter name here differs from its implementation in targets_impl.go; aligning the names would enhance clarity.
UpdateTargetsCancelStatus(ctx context.Context, questionnaireID int, targets []string, remindStatus bool) error
model/targets_test.go:421
- [nitpick] Consider providing a unique description for each test case to avoid ambiguity when diagnosing test failures.
description: "both valid and invalid targets changed order",
…indStatus to cancelStatus for consistency
Closes #1297