Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't show the contact if the email is incorrect #2087

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

NicolasBourdin88
Copy link
Contributor

@NicolasBourdin88 NicolasBourdin88 commented Oct 30, 2024

Do not allow the user to add a contact with an invalid email, and set only one entry as 'ContactChipAdapter' for easier management.

@NicolasBourdin88 NicolasBourdin88 added bug Something isn't working quick A pull request consisting of a few lines labels Oct 30, 2024
@NicolasBourdin88 NicolasBourdin88 self-assigned this Oct 30, 2024
Copy link

sonarcloud bot commented Oct 30, 2024

@@ -380,7 +381,7 @@ class RecipientFieldView @JvmOverloads constructor(
fun initRecipients(initialRecipients: List<Recipient>, otherFieldsAreAllEmpty: Boolean = true) {

initialRecipients.forEach { recipient ->
if (contactChipAdapter.addChip(recipient)) contactAdapter.addUsedContact(recipient.email)
addRecipient(recipient.email, recipient.name)
Copy link
Contributor

@LunarX LunarX Nov 5, 2024

Choose a reason for hiding this comment

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

I just realized that this breaks the externals signalization of chips. Flagging recipients as "manuallyEntered" should be less error prone to avoid this kind of bugs.

In situations like this where split code is merged as one, you should always try and follow all of the logic from the very start of the code execution up to the very end. You'd need to understand precisely what differs in both ways of doing so you can be certain that it is safe to merge the two logics. This one was pretty hard to detect though if you didn't know about external recipients

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working quick A pull request consisting of a few lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants