Skip to content

fix(ui): Omitted profiles shown in group list while setting group up#1568

Open
Sotatek-DukeVu wants to merge 1 commit intodevelopfrom
VT20-2574-omitted-profiles-shown-in-group-list-while-setting-group-up
Open

fix(ui): Omitted profiles shown in group list while setting group up#1568
Sotatek-DukeVu wants to merge 1 commit intodevelopfrom
VT20-2574-omitted-profiles-shown-in-group-list-while-setting-group-up

Conversation

@Sotatek-DukeVu
Copy link
Collaborator

@Sotatek-DukeVu Sotatek-DukeVu commented Jan 30, 2026

Description

Omitted profiles shown in group list while setting group up

Checklist before requesting a review

Issue ticket number and link

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.
Screen.Recording.2026-01-30.at.14.44.10.mov

@Sotatek-DukeVu Sotatek-DukeVu self-assigned this Jan 30, 2026
@jimcase
Copy link
Contributor

jimcase commented Feb 4, 2026

I got an issue @Sotatek-DukeVu
In the initiator wallet when creating a group identifier 3/3:

  1. Go the confirm screen.
  2. Set Required signers to 3 members.
  3. Set Recovery signers to 3 memebrs.
  4. Edit group member by removing 1 member.
  5. The Required signers and Recovery signer still says "3 members".
  6. I click on Send requests.
  7. I get the error toast "Unable to create group"

The Required signers and Recovery signers values should be reset(or signers form prompted, see figma designs) and Send requests button should be disabled.

Tested on real devices: Android 15 and iOS26

@Sotatek-DukeVu
Copy link
Collaborator Author

I got an issue @Sotatek-DukeVu In the initiator wallet when creating a group identifier 3/3:

  1. Go the confirm screen.
  2. Set Required signers to 3 members.
  3. Set Recovery signers to 3 memebrs.
  4. Edit group member by removing 1 member.
  5. The Required signers and Recovery signer still says "3 members".
  6. I click on Send requests.
  7. I get the error toast "Unable to create group"

The Required signers and Recovery signers values should be reset(or signers form prompted, see figma designs) and Send requests button should be disabled.

Tested on real devices: Android 15 and iOS26

@jimcase It has been fixed in this PR: #1564.

@Sotatek-DukeVu Sotatek-DukeVu force-pushed the VT20-2574-omitted-profiles-shown-in-group-list-while-setting-group-up branch from ae82485 to de7d46a Compare February 5, 2026 02:29
@jimcase
Copy link
Contributor

jimcase commented Feb 5, 2026

I got an issue @Sotatek-DukeVu In the initiator wallet when creating a group identifier 3/3:

  1. Go the confirm screen.
  2. Set Required signers to 3 members.
  3. Set Recovery signers to 3 memebrs.
  4. Edit group member by removing 1 member.
  5. The Required signers and Recovery signer still says "3 members".
  6. I click on Send requests.
  7. I get the error toast "Unable to create group"

The Required signers and Recovery signers values should be reset(or signers form prompted, see figma designs) and Send requests button should be disabled.
Tested on real devices: Android 15 and iOS26

@jimcase It has been fixed in this PR: #1564.

Okey, so please, merge develop branch to this PR so we can test it properly.

@iFergal
Copy link
Collaborator

iFergal commented Feb 5, 2026

@jimcase Duke has force pushed, so I think the changes are there. I tested and it works.

However, after step 4, what are we doing - pressing back? The changes seem to take affect so I think there's something wrong with the other PR. I will check over there in that ticket.

From my POV, this PR resolves the related bug.


if (groupDetails) {
memberData = memberData.filter((connection) => {
return groupDetails?.members.some(
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove ?

identity?.id,
]);

const getMemberConnections = useCallback(
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 it would be cleaner to have 2 functions - one that accepts GroupInformation and one that accepts MultiSigIcpRequestDetails

let memberData = [...connections];

if (groupDetails) {
memberData = memberData.filter((connection) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

groupDetails has the aid and name of each member in the group. Why do we need to filter connections - are the extra fields of ConnectionShortDetails actually used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason I filter connections here is because this data will be set back into the initialState.selectedConnections state in SetupGroupProfile. This state is then passed down to PendingGroup.

Basically, this state is where we store the connections that were scanned and selected during the setup screen. At this point, we only need a single function to display the member list.

});
}

if (multisigIcpDetails && isPendingMember) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why can't we also just use multisigIcpDetails.otherConnections and multisigIcpDetails.sender directly? These are already in the right format of MultisigConnectionDetails which extends ConnectionShortDetails

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

if (
memberConnections.length == state.selectedConnections.length &&
state.selectedConnections.every((item) =>
memberConnections.some((m) => m.contactId == item.contactId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

also don't see the need for this extra if statement to do a diff. Faster and easier to just remove this (and on line 340)

selectedConnections: currentProfile.multisigConnections,
selectedConnections: isPendingState
? []
: currentProfile.multisigConnections,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

currentProfile.multisigConnections currently contains members who were not selected. Because of that, it would be confusing if the screen first displayed the full list of users in the group and then removed the users who are not participating. Therefore, I will set this prop to empty and populate it again after the core function returns the correct data.
On the UI, it will initially display an empty state, and after loading finishes, it will show the member list.

mainContent={`${i18n.t(
`setupgroupprofile.initgroup.setsigner.members`,
`setupgroupprofile.initgroup.setsigner.${
(signingThreshold || 0) > 1 ? "members" : "member"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Image

These don't exist...

Also, before I initiate, I still see "1 members" - not sure if another PR fixes that

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