Conversation
|
@sahara2006 I've opened a new pull request, #698, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
This PR refactors duplicate logic in the Application section by extracting common patterns into reusable composables. The main changes follow the established pattern from the partition feature, creating a unified approach for managing edit modes and update operations.
Key changes:
- Created
useApplicationInformationcomposable to centralize application editing logic - Renamed
EditModetoPartitionEditModefor clarity and consistency - Refactored
ApplicationContent.vueto use the new composable pattern with props and events
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/features/application/store.ts |
Adds helper function createDefaultApplicationSeed() and an incomplete store definition that needs to be removed |
src/components/partitionDetail/composables/usePartitionInformation.ts |
Renames EditMode type to PartitionEditMode for better clarity, but missing import for usePartitionStore |
src/components/partitionDetail/PartitionName.vue |
Updates type import to use renamed PartitionEditMode |
src/components/partitionDetail/PartitionGroup.vue |
Updates type import to use renamed PartitionEditMode |
src/components/partitionDetail/PartitionBudget.vue |
Updates type import to use renamed PartitionEditMode |
src/components/applicationDetail/composables/useApplicationInformation.ts |
New composable following the partition pattern, but contains incorrect error message referencing "partition" instead of "application" |
src/components/applicationDetail/ApplicationLogs.vue |
Integrates the new useApplicationInformation composable |
src/components/applicationDetail/ApplicationContent.vue |
Major refactor to use props/events pattern, but has duplicate imports and missing destructured functions from store |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hasAuthority = computed(() => { | ||
| if (!application.value) return false | ||
| return useApplication(application.value).isApplicationCreator.value(me.value) | ||
| }) |
There was a problem hiding this comment.
The hasAuthority computed property calls useApplication() on every evaluation. This composable should be called once at setup time, not inside a computed property. Consider moving the useApplication call outside the computed and storing isApplicationCreator in a ref or computed that references it.
重複する記述を削除 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sahara2006
left a comment
sahara2006
left a comment
There was a problem hiding this comment.
handleUpdateTargetの検討
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
#474 送信などのロジックをusePartitionInformation.tsに分離