Affiliation apply to all projects #2673
Conversation
WalkthroughThe changes involve structural modifications to the layout of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dropdown
participant AffiliationManager
User->>Dropdown: Click "more" icon
Dropdown->>User: Show options ("Apply to all projects", "Delete affiliation")
User->>Dropdown: Select "Apply to all projects"
Dropdown->>AffiliationManager: Call copyToOtherProjects()
AffiliationManager-->>Dropdown: Update affiliations
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
frontend/src/modules/contributor/components/edit/affilations/contributor-affilations-edit-item.vue (4)
Line range hint
67-71: Fix incorrect validation binding for end date picker.The end date picker's error class is incorrectly bound to the start date's validation state:
:class="$v.dateStart.$invalid && $v.dateStart.$dirty ? 'is-error' : ''"Apply this fix:
-:class="$v.dateStart.$invalid && $v.dateStart.$dirty ? 'is-error' : ''" +:class="$v.dateEnd.$invalid && $v.dateEnd.$dirty ? 'is-error' : ''"
Line range hint
93-98: Consider externalizing validation messages for i18n support.The hardcoded error messages should be moved to a translation system for better maintainability and internationalization support.
Consider using Vue i18n:
-:error-messages="{ - required: 'This field is required', - minDate: 'This date range is not valid', -}" +:error-messages="{ + required: $t('validation.required'), + minDate: $t('validation.dateRange.invalid'), +}"
Line range hint
115-121: Enhance type safety for form interface.The
AffilationForminterface could be improved with more specific types and required fields.Consider this enhancement:
export interface AffilationForm { - segmentId: string; - organization: string | null, - dateStart: string; - dateEnd: string; - currentlyAffiliated: boolean; + segmentId: string; + organization: string | null; + dateStart: string; // Consider using Date type + dateEnd: string | null; // Optional when currentlyAffiliated is true + currentlyAffiliated: boolean; + readonly id?: string; // For existing affiliations }
Line range hint
61-71: Consider making date format configurable.The date format is hardcoded as "MMM YYYY". Consider making it configurable to support different format preferences.
Consider adding a prop:
+const props = defineProps<{ + modelValue: AffilationForm, + contributor: Contributor, + dateFormat?: string, // Add format prop +}>(); <el-date-picker ... - format="MMM YYYY" + :format="props.dateFormat || 'MMM YYYY'" ... />frontend/src/modules/contributor/components/edit/affilations/contributor-affilations-edit.vue (1)
57-78: Consider adding confirmation dialogs and loading states.The dropdown menu implementation looks good, but consider these UX improvements:
- Add a confirmation dialog for the delete action to prevent accidental deletions
- Add loading states to buttons during operations
Example implementation:
<lf-dropdown-item type="danger" + :disabled="isLoading" @click="form.splice(ai, 1)" > <lf-icon-old name="delete-bin-6-line" /> Delete affiliation </lf-dropdown-item>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
frontend/src/modules/contributor/components/edit/affilations/contributor-affilations-edit-item.vue(2 hunks)frontend/src/modules/contributor/components/edit/affilations/contributor-affilations-edit.vue(3 hunks)
🔇 Additional comments (2)
frontend/src/modules/contributor/components/edit/affilations/contributor-affilations-edit-item.vue (1)
3-3: LGTM! Width adjustments improve the layout balance.
The asymmetric width distribution (5/12 for organization selector, 7/12 for date pickers) provides better space utilization, giving more room for the date picker controls which typically need more horizontal space.
Also applies to: 52-52
frontend/src/modules/contributor/components/edit/affilations/contributor-affilations-edit.vue (1)
122-123: LGTM!
The dropdown component imports follow the project's conventions and are properly organized.
| const copyToOtherProjects = (index: number) => { | ||
| const affiliation = form.value[index]; | ||
| const otherProjects = props.contributor.segments.filter((seg) => seg.id !== affiliation.segmentId); | ||
|
|
||
| otherProjects.forEach((project) => { | ||
| form.value.push({ | ||
| segmentId: project.id, | ||
| organization: affiliation.organization, | ||
| dateStart: affiliation.dateStart, | ||
| dateEnd: affiliation.dateEnd, | ||
| currentlyAffiliated: affiliation.currentlyAffiliated, | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Add validation and error handling to copyToOtherProjects.
The method successfully copies affiliations but could benefit from additional safeguards:
- Validate if similar affiliations already exist in target projects
- Add error handling and loading state management
- Provide user feedback about the operation's success/failure
Here's a suggested implementation:
-const copyToOtherProjects = (index: number) => {
+const copyToOtherProjects = async (index: number) => {
+ try {
+ sending.value = true;
const affiliation = form.value[index];
const otherProjects = props.contributor.segments.filter((seg) => seg.id !== affiliation.segmentId);
+ // Check for existing similar affiliations
+ const duplicates = otherProjects.filter(project =>
+ form.value.some(existing =>
+ existing.segmentId === project.id &&
+ existing.organization === affiliation.organization &&
+ existing.dateStart === affiliation.dateStart
+ )
+ );
+
+ if (duplicates.length > 0) {
+ Message.warning(`Similar affiliations already exist in ${duplicates.length} project(s)`);
+ return;
+ }
+
otherProjects.forEach((project) => {
form.value.push({
segmentId: project.id,
@@ -206,6 +223,13 @@
currentlyAffiliated: affiliation.currentlyAffiliated,
});
});
+
+ Message.success(`Affiliation copied to ${otherProjects.length} project(s)`);
+ } catch (error) {
+ Message.error('Failed to copy affiliations');
+ } finally {
+ sending.value = false;
+ }
};Committable suggestion skipped: line range outside the PR's diff.
Changes proposed ✍️
What
copilot:summary
copilot:poem
Why
How
copilot:walkthrough
Checklist ✅
Feature,Improvement, orBug.Summary by CodeRabbit
New Features
Style
Bug Fixes