Skip to content

Conversation

@tomrndom
Copy link

ref: https://app.clickup.com/t/86b6jhctw

Signed-off-by: Tomás Castillo [email protected]

@tomrndom tomrndom requested a review from smarcet September 25, 2025 18:06
@tomrndom tomrndom changed the title Fix/new speaker form fields fix: update fields on new speaker member select Sep 25, 2025
@smarcet smarcet requested a review from Copilot January 5, 2026 18:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the conditional logic for when speaker fields should be replaced with member information during member selection in the speaker form. The fix ensures that for new speakers (entity.id === 0), the fields are always updated when a member is selected, regardless of the global configuration setting.

  • Modified conditional logic to handle new speaker member selection
  • Ensures member information populates fields for new speakers even when the global replacement setting is disabled

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +85 to +88
if (
(value && mustReplaceSpeakerFieldsWithMemberInfo()) ||
(value && !mustReplaceSpeakerFieldsWithMemberInfo() && entity.id === 0)
) {
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional logic can be simplified. The current condition checks 'value' twice and calls 'mustReplaceSpeakerFieldsWithMemberInfo()' twice. This can be refactored to: 'value && (mustReplaceSpeakerFieldsWithMemberInfo() || entity.id === 0)' which is more readable and efficient.

Suggested change
if (
(value && mustReplaceSpeakerFieldsWithMemberInfo()) ||
(value && !mustReplaceSpeakerFieldsWithMemberInfo() && entity.id === 0)
) {
if (value && (mustReplaceSpeakerFieldsWithMemberInfo() || entity.id === 0)) {

Copilot uses AI. Check for mistakes.
Copy link

@smarcet smarcet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smarcet smarcet merged commit 792d75e into master Jan 5, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants