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

Contacts after updating #1337

Closed
wants to merge 5 commits into from
Closed

Contacts after updating #1337

wants to merge 5 commits into from

Conversation

azackmatoff
Copy link
Collaborator

No description provided.

@azackmatoff azackmatoff self-assigned this Jul 4, 2023
@azackmatoff azackmatoff added A1-bugfix PR fixes a bug B1-medium Elevates a release containing this PR to "medium priority" C0-breaksnothing PR does not introduce any breaking changes labels Jul 4, 2023
Copy link
Member

@clangenb clangenb left a comment

Choose a reason for hiding this comment

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

Nice, I was not expecting that fix! I only have one tiny remark, and one question.

Comment on lines +107 to +113
final name = Community.fromCid(appStore.encointer.community?.cid.toFmtString()).name;
if (name == 'No Name' || name.toLowerCase().contains('no name')) {
return context.l10n.unknown;
} else {
return name;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I am quite sure that the community can never have this No Name. Do you have information that I am wrong?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked for community and couldn't see anything with No Name names))), but still wasn't sure so added this code.

Comment on lines +47 to +52
if (contact.address == counterParty) {
if (contact.name == 'No Name' || contact.name.toLowerCase().contains('no name')) {
return context.l10n.unknown;
}
return contact.name;
}
Copy link
Member

Choose a reason for hiding this comment

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

Oh wow, so the no names, were actually because there were accounts that had No Name as a name because it was the default choice? This was a tricky one. XD

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did go through codes and couldn't find anyplace where No Name would come up, so thought maybe it's here)))

@clangenb
Copy link
Member

The actual fix is here: #1376

@clangenb clangenb closed this Jul 15, 2023
@clangenb clangenb deleted the Contacts-after-updating branch August 3, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A1-bugfix PR fixes a bug B1-medium Elevates a release containing this PR to "medium priority" C0-breaksnothing PR does not introduce any breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants