Merged
Conversation
|
共有: oapi-codegenのサーバー生成で使われるUUIDはgoogle/uuidだったはずなのでgofrs/uuidを使うときは注意する必要がありそうです |
This was
linked to
issues
Jun 23, 2024
Closed
Closed
Closed
Closed
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1221 +/- ##
==========================================
- Coverage 63.88% 63.81% -0.07%
==========================================
Files 27 26 -1
Lines 4004 4024 +20
==========================================
+ Hits 2558 2568 +10
- Misses 1062 1070 +8
- Partials 384 386 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…reInfo to follow the order of their top response under the current sort parameter
一時保存されたアンケートのバリデーションを緩める
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
選択式問題の答えを、controller層で選択肢番号から内容に変換
…tools_update oapi-codegenのファイルをgo generateで自動生成可能に＆tools.goからgo get -toolへの移行
Contributor
There was a problem hiding this comment.
Pull request overview
This PR represents a major architectural shift from a custom router implementation to an OpenAPI-based approach. The changes involve removing the existing router handlers and test files while introducing auto-generated OpenAPI types and specifications. Several model files have been updated to support the new architecture, with adjustments to function signatures and interface definitions.
Reviewed changes
Copilot reviewed 79 out of 91 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| router/results.go | Removed entire file containing result handler implementation |
| router/responses_test.go | Removed comprehensive test suite for response handlers |
| router/responses.go | Removed response-related HTTP handlers and validation logic |
| router/questions_test.go | Removed extensive test coverage for question validation and handling |
| router/questions.go | Removed question editing and deletion handlers |
| router/questionnaires_test.go | Removed detailed test suite for questionnaire operations |
| router/questionnaires.go | Removed all questionnaire CRUD handlers and helper functions |
| router/api_test.go | Removed test utilities and helper functions |
| router/api.go | Removed API struct and constructor |
| router.go | Removed main routing configuration |
| openapi/types.go | Added auto-generated OpenAPI type definitions (1846 lines) |
| openapi/spec.go | Added auto-generated OpenAPI specification loader |
| openapi/generate.go | Added code generation directives for OpenAPI |
| model/validations_test.go | Updated function calls to match new signature with additional parameters |
| model/validations_impl.go | Refactored variable declarations for better code quality |
| model/transaction_mock.go | Updated mock to remove unused sql.TxOptions parameter |
| model/transaction.go | Added mockgen directive for code generation |
| model/targets.go | Extended interface with three new methods for targeting functionality |
| .spectral.yaml | Added OpenAPI linting configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
155
to
+157
| if MinBound != "" { | ||
| min, err := strconv.ParseFloat(MinBound, 64) | ||
| minBoundNum = min | ||
| minBoundNum, err = strconv.ParseFloat(MinBound, 64) |
There was a problem hiding this comment.
The variable err is declared once and reused for both MinBound and MaxBound parsing. While this works, if the first parsing succeeds and the second fails, the error handling at line 164 may be confusing since the variable name doesn't indicate which bound failed. Consider using distinct variable names or scoping the err variable to each if block for clarity.
| type MockTransaction struct{} | ||
|
|
||
| func (m *MockTransaction) Do(ctx context.Context, txOption *sql.TxOptions, f func(ctx context.Context) error) error { | ||
| func (m *MockTransaction) Do(ctx context.Context, f func(ctx context.Context) error) error { |
There was a problem hiding this comment.
The removal of the txOption *sql.TxOptions parameter is a breaking change to the ITransaction interface. Ensure that all implementations of this interface have been updated accordingly, and that any code calling this method has been adjusted to match the new signature.
…openapi-dev-url fix: add development server URL to swagger configuration
When PostMessage was called inside the DB transaction, a failure on the second or later chunk would roll back the transaction, but any previously sent chunks already contained a link to a questionnaire that no longer exists. By calling PostMessage after the transaction commits, the linked questionnaire is guaranteed to exist. Send failures are only logged; the questionnaire creation itself is still treated as successful. https://claude.ai/code/session_01CgXuuWWwhkMr1fXAR9LGgw
…ai2Y feat: 対象者が多い場合のtraQメッセージ分割処理
…ount-fixes fix: fix some bugs
Add an optional isPublished query parameter to GET /questionnaires that filters questionnaires by their is_published field: - true: return only published questionnaires - false: return only unpublished questionnaires - omitted: return all questionnaires regardless of published state This replaces the previous hardcoded filter that always restricted results to published questionnaires only. https://claude.ai/code/session_015TFFD6ovua8GnmjqbiZtDh
When isPublished is false, additionally require the user to be an administrator of the questionnaire via EXISTS subquery on the administrators table. This prevents non-admin users from discovering unpublished questionnaires. When isPublished is omitted (nil), restore the previous safe default of only returning published questionnaires (is_published IS TRUE), reverting the unintentional security regression from the initial implementation. Behavior summary: - isPublished omitted: published only (safe default, same as before) - isPublished=true: published only - isPublished=false: unpublished AND user is questionnaire admin https://claude.ai/code/session_015TFFD6ovua8GnmjqbiZtDh
… questionnaires Rename the questionnaire filter parameter from isPublished to isDraft to be consistent with the response-level isDraft convention: - isDraft=true: show only unpublished (draft) questionnaires - isDraft=false: show only published questionnaires - omitted: show only published questionnaires (safe default) Enforce that isDraft=true implicitly sets onlyAdministratedByMe=true in the controller, ensuring non-admin users cannot discover unpublished questionnaires. Also verified that GET /questionnaires/:questionnaireID is already protected: QuestionnaireReadAuthenticate middleware returns 403 when the questionnaire is not published and the user is neither a system admin nor a questionnaire admin. https://claude.ai/code/session_015TFFD6ovua8GnmjqbiZtDh
When isDraft is omitted, show both published questionnaires and unpublished questionnaires that the user administrates, consistent with the API convention that omitting a filter means no restriction. Previously, omitting isDraft incorrectly defaulted to published-only (same as isDraft=false). Now: - isDraft omitted: published questionnaires OR drafts the user administrates - isDraft=true: draft (unpublished) questionnaires only (admin-restricted) - isDraft=false: published questionnaires only Update swagger and types.go comments to reflect the corrected behavior. https://claude.ai/code/session_015TFFD6ovua8GnmjqbiZtDh
…lished-filter-qF4nP Add isDraft filter to GET /questionnaires endpoint
DeleteResponse was calling DeleteRespondent and DeleteResponse without a transaction, which could leave data in an inconsistent state if one operation succeeded and the other failed. Wrap both calls in ITransaction.Do() to ensure atomicity. Fixes #1520 https://claude.ai/code/session_01VaoSLHjZqHPryFAE1pF15y
…esponse processing - Add nil checks for MinLabel/MaxLabel in EditQuestionnaire before dereferencing pointers (was already done in PostQuestionnaire) - Add bounds check for ScaleLabels slice before accessing index 0 in convertQuestions to prevent index out of range panic - Return HTTP 500 when scale label is missing from map during response validation instead of silently using zero-value bounds (ScaleMin=0, ScaleMax=0) which would incorrectly reject valid answers - Pass a proper error message to echo.NewHTTPError when questionnaire deadline has passed, instead of nil err
…port-Ik4nN fix: scale label nil dereference, out of bounds panic, and error handling bugs
…-remindertargets fix: move migration of new table ReminderTargets to v3
Wrap response deletion in transaction for data consistency
Contributor
Author
|
それぞれについてレビューしていたので問題なしとする |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
新機能
更新