fix: 3 issues at once by JoaoVitorLobo · Pull Request #552 · sinfo/deck2

JoaoVitorLobo · 2026-01-24T23:56:00Z

Create speaker with contact:
- ContactForm.vue
- CreateSpeakerForm.vue
- speaker.go
Remove pending status from communications:
- Communications.vue
Gender on templates:
- 33-pt.html
- BulkEmailDialogTrigger.vue
- SpeakerCommunications.vue
- useBulkEmails.ts
- useDirectEmail.ts
- speakers.ts
- templates.ts
- ResponsibilitiesView.vue

Francisca105

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.

Francisca105 · 2026-01-25T14:36:56Z

frontend/src/components/Communications.vue

                </div>

-                <div class="flex items-center gap-1">
+                <!-- Pending Status -->


Avoid leaving commented-out code, if it’s not needed, please remove it.

Francisca105 · 2026-01-25T14:37:09Z

frontend/src/components/Communications.vue

 };

-const getStatusLabel = (status: ThreadStatus): string => {
+/*const getStatusLabel = (status: ThreadStatus): string => {


Avoid leaving commented-out code, if it’s not needed, please remove it.

Francisca105 · 2026-01-25T15:04:31Z

frontend/src/components/companies/ContactForm.vue

      <!-- Gender Field -->
      <div class="space-y-2">
-        <Label class="text-sm font-medium">Gender</Label>
+        <Label class="text-sm font-medium">Gender *</Label>


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.

Francisca105 · 2026-01-25T15:07:19Z

frontend/src/components/speakers/CreateSpeakerForm.vue

        </Button>
        <div class="flex gap-2">
-          <Button :disabled="isLoading" @click="createSpeakerAndFinish">
+          <Button


Step 3 relies on ContactForm, adding the required validation there makes the step 3 validation redundant. These changes in this file can be removed.

Francisca105 · 2026-01-25T15:10:02Z

frontend/src/components/speakers/SpeakerCommunications.vue

+  const memberGender = authStore.member!.contactObject.gender;
+  const speakerGender = props.speaker.contactObject.gender;
+
+  // Use contactObject which contains the full Contact (including gender)


Suggested change

// Use contactObject which contains the full Contact (including gender)

These comments are unnecessary and can be removed.

Francisca105 · 2026-01-25T15:10:13Z

frontend/src/components/speakers/SpeakerCommunications.vue

+  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


Suggested change

// Fallback to empty string when gender is unavailable to avoid runtime errors

These comments are unnecessary and can be removed.

Francisca105 · 2026-01-25T15:14:36Z

frontend/src/lib/templates.ts

 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",


The templates were reverted to the previous edition, please undo it.

Francisca105 · 2026-01-25T15:16:08Z

frontend/src/lib/templates.ts

+    case "OTHER":
+      return { article: "e", suffix: "e" };
+    default:
+      return { article: "o/a/e", suffix: "(a)" };


It would be better if the default case was equal to the "OTHER" case.

Francisca105

Suggestion for #552 (comment)

Francisca105 · 2026-01-25T15:22:19Z

frontend/src/components/companies/ContactForm.vue

Suggested change

const hasGender = !!formData.contact.gender;

const hasLanguage = !!formData.contact.language;

return hasName && hasEmail && hasGender && hasLanguage;

Francisca105 · 2026-01-25T15:23:05Z

frontend/src/components/companies/ContactForm.vue

 });

 const validationMessage = computed(() => {
  if (!props.withoutName && !formData.name?.trim()) return "Name is required";


Suggested change

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";

JoaoVitorLobo · 2026-01-28T16:43:11Z

Before, there was no validation nor validation message in the Create Speaker – Step 3:

watch was emitting every change from ContactForm to CreateSpeakersForm, so the validation was never being executed as supposed inside ContactForm
the validation code available in ContactForm only supported the edit contact feature, not create contact
if CreateSpeakersForm received an invalid newData, the validation would be supposed to be executed in the CreateSpeakersForm, so it became necessary to emit newData only when it is valid, executing the validation on ContactFOrm
Added hasGender and hasLanguage to both validation and validationMessage
Even though validation and validation message are now handled in ContactForm, it was still necessary to disable the Create Speaker button in CreateSpeakersForm when Step 3 is invalid. Otherwise, if ContactForm first emmited a valid data and later emmited invalid data, the Create Speaker button would remain enabled for invalid data

jovitor1898 added 2 commits January 24, 2026 23:44

fix: 3 issues at once

4bf5b2f

fix: 3 issues at once

0479791

Francisca105 changed the base branch from master to staging January 25, 2026 14:33

Francisca105 requested changes Jan 25, 2026

View reviewed changes

jovitor1898 added 3 commits January 27, 2026 22:44

fix: comments and template number

21b6855

fix: validation in ContactForm

76add31

fix: validation in ContactForm

9a0e2e8

+  const hasGender = !!formData.contact.gender;
+  const hasLanguage = !!formData.contact.language;
+  return hasName && hasEmail && hasGender && hasLanguage;

+  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";

Conversation

JoaoVitorLobo commented Jan 24, 2026

Uh oh!

Francisca105 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Francisca105 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoaoVitorLobo commented Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants