fix: 3 issues at once #552
Conversation
Francisca105
left a comment
Francisca105
left a comment
There was a problem hiding this comment.
A couple of thoughts on good practices: try to keep changes focused on a single issue, with one PR per issue, and mention the related issue in the PR description so reviewers have full context.
| </div> | ||
|
|
||
| <div class="flex items-center gap-1"> | ||
| <!-- Pending Status --> |
There was a problem hiding this comment.
Avoid leaving commented-out code, if it’s not needed, please remove it.
| }; | ||
| const getStatusLabel = (status: ThreadStatus): string => { | ||
| /*const getStatusLabel = (status: ThreadStatus): string => { |
There was a problem hiding this comment.
Avoid leaving commented-out code, if it’s not needed, please remove it.
| <!-- Gender Field --> | ||
| <div class="space-y-2"> | ||
| <Label class="text-sm font-medium">Gender</Label> | ||
| <Label class="text-sm font-medium">Gender *</Label> |
There was a problem hiding this comment.
The asterisk in the label 'Gender *' indicates this field should be required, but there is no validation or enforcement in the code to support that.
| </Button> | ||
| <div class="flex gap-2"> | ||
| <Button :disabled="isLoading" @click="createSpeakerAndFinish"> | ||
| <Button |
There was a problem hiding this comment.
Step 3 relies on ContactForm, adding the required validation there makes the step 3 validation redundant. These changes in this file can be removed.
| const memberGender = authStore.member!.contactObject.gender; | ||
| const speakerGender = props.speaker.contactObject.gender; | ||
| // Use contactObject which contains the full Contact (including gender) |
There was a problem hiding this comment.
| // Use contactObject which contains the full Contact (including gender) |
These comments are unnecessary and can be removed.
| const speakerGender = props.speaker.contactObject.gender; | ||
| // Use contactObject which contains the full Contact (including gender) | ||
| // Fallback to empty string when gender is unavailable to avoid runtime errors |
There was a problem hiding this comment.
| // Fallback to empty string when gender is unavailable to avoid runtime errors |
These comments are unnecessary and can be removed.
| export const templatePaths: Record<EmailTemplate, string> = { | ||
| [EmailTemplate.COMPANIES_EN]: "/companies/33-en.html", | ||
| [EmailTemplate.COMPANIES_PT]: "/companies/33-pt.html", | ||
| [EmailTemplate.COMPANIES_EN]: "/companies/32-en.html", |
There was a problem hiding this comment.
The templates were reverted to the previous edition, please undo it.
| case "OTHER": | ||
| return { article: "e", suffix: "e" }; | ||
| default: | ||
| return { article: "o/a/e", suffix: "(a)" }; |
There was a problem hiding this comment.
It would be better if the default case was equal to the "OTHER" case.
Francisca105
left a comment
Francisca105
left a comment
There was a problem hiding this comment.
Suggestion for #552 (comment)
| const isValid = computed(() => { | ||
| const hasName = props.withoutName || formData.name?.trim(); | ||
| const hasEmail = formData.contact.mails.some((mail) => mail.mail.trim()); | ||
| return hasName && hasEmail; |
There was a problem hiding this comment.
| const hasGender = !!formData.contact.gender; | |
| const hasLanguage = !!formData.contact.language; | |
| return hasName && hasEmail && hasGender && hasLanguage; |
| }); | ||
| const validationMessage = computed(() => { | ||
| if (!props.withoutName && !formData.name?.trim()) return "Name is required"; |
There was a problem hiding this comment.
| if (!props.withoutName && !formData.name?.trim()) return "Name is required"; | |
| if (!formData.contact.gender) return "Gender is required"; | |
| if (!formData.contact.language) return "Language is required"; |
- ContactForm.vue
- CreateSpeakerForm.vue
- speaker.go
- Communications.vue
- 33-pt.html
- BulkEmailDialogTrigger.vue
- SpeakerCommunications.vue
- useBulkEmails.ts
- useDirectEmail.ts
- speakers.ts
- templates.ts
- ResponsibilitiesView.vue