Skip to content

Conversation

@Sotatek-DucPhung
Copy link
Collaborator

@Sotatek-DucPhung Sotatek-DucPhung commented Oct 22, 2025

Cleaner way to provide group name to UI

Description

Renamed group metadata username to proposedUsername, introduced groupUsername/groupId, and stopped persisting groupMetadata on group identifiers; all agent services, migrations, utilities, fixtures, tests, and event payloads now reflect the new schema.
Updated UI selectors, components, and profile state handling to read groupUsername or member metadata, adjusted pending-group modals, and refreshed fixtures/tests accordingly.

Checklist before requesting a review

Issue ticket number and link

  • This PR has a valid ticket number or issue: VT20-2166

Testing & Validation

  • This PR has been tested/validated in iOS, Android and browser.
  • Added new unit tests, if relevant.

Design Review

  • In case this PR contains changes to the UI, add some screenshots and/or videos to show the changes on relevant devices.

@Sotatek-DucPhung Sotatek-DucPhung marked this pull request as draft October 23, 2025 18:50
@Sotatek-DucPhung Sotatek-DucPhung force-pushed the feature/VT20-2166-cleaner-way-to-provide-group-name branch from e8f0d6e to 89d77c7 Compare October 28, 2025 03:43
@Sotatek-DucPhung Sotatek-DucPhung marked this pull request as ready for review October 28, 2025 04:09
@Sotatek-DucPhung
Copy link
Collaborator Author

@iFergal Please ignore the previous review comments, as I’ve force-pushed a reimplementation.

@Sotatek-DucPhung Sotatek-DucPhung marked this pull request as draft November 4, 2025 10:48
@Sotatek-DucPhung Sotatek-DucPhung marked this pull request as ready for review November 5, 2025 09:03
dispatch(addGroupProfileAsync(event.payload.group));
await dispatch(addGroupProfileAsync(event.payload.group));

// Refresh multisig connections cache after group is created
Copy link
Collaborator

Choose a reason for hiding this comment

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

addGroupProfile already sets multisigConnections: group.multisigConnections - we shouldn't need these changes. Can you check why it's not working there?

if (profile.groupMemberPre && identifiers[profile.groupMemberPre]) {
groupIdToFilter =
identifiers[profile.groupMemberPre].groupMetadata?.groupId;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

const groupIdToFilter = profile.groupMemberPre ? identifiers[profile.groupMemberPre]?.groupMetadata?.groupId : profile.groupMetadata?.groupId; would work to simplify.

);
const storedIdentifiers = await Agent.agent.identifiers.getIdentifiers();
const allIdentifiersIncludingMember =
await Agent.agent.identifiers.getIdentifiers(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there is a general refactor we should dod here for filtering. But I would do it in another PR. Leaving this comment as a reminder for me.


const isMember = !identity?.groupMetadata?.groupInitiator;
const isPendingMember = isMember && initGroupNotification;
const isPendingMember = !!initGroupNotification;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what this variable is meant to be doing but we cannot rely on the notification alone because after the notification is deleted, the group may still be pending. @Sotatek-DukeVu please comment


const getInceptionStatus = useCallback(async () => {
if (!identity?.id || !identity?.groupMetadata) return;
// We can fetch inception status using gHab id even if groupMetadata is not stored on gHab
Copy link
Collaborator

Choose a reason for hiding this comment

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

This explains the change but isn't needed to be kept in the code since new devs will never know we had it there. lets remove this one!

? cardData.groupUsername ||
cardData.groupMetadata?.proposedUsername ||
""
: member);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Presumably isCurrent means if this member equals us, right?

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.

5 participants