Skip to content

Add ability to see main members in groups #312

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

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

Conversation

TGODiamond
Copy link

@TGODiamond TGODiamond commented Mar 13, 2025

Adds the ability to see main members in groups. (Inverse of a user's main group)

Also fixes the translation file lxqt-admin-user_kab.ts
Edit: This can be done by @stefonarch instead.

As Im new to C++, I just want to make sure this is safe and good:

// add groups to this user and add this user to their group's main members
for(GroupInfo* group: mGroups) { // Removed the "std::as_const(mGroups)", replaced with "mGroups"
    if(group->hasMember(user->name()))
        user->addGroup(group->name());
    if(user->gid() == group->gid())
        group->addMainMember(user->name());
}

Screenshots

Screenshot_20250313_212956
Screenshot_20250313_213031
When adding a new group:
Screenshot_20250313_214818

@tsujan
Copy link
Member

tsujan commented Mar 13, 2025

Please remove the changes to translations from your PR.

@TGODiamond
Copy link
Author

Oh, are they automated?

@TGODiamond
Copy link
Author

It is mostly correcting the line numbers, and adding the new, missing entries.

@tsujan
Copy link
Member

tsujan commented Mar 13, 2025

Translation updates are done separately, by @stefonarch . Code PRs shouldn't touch translation files.

@TGODiamond
Copy link
Author

Done!

@tsujan tsujan requested a review from palinek March 13, 2025 21:13
@tsujan
Copy link
Member

tsujan commented Mar 13, 2025

Thanks for the PR and also for cleaning it up. I requested a review.

@palinek
Copy link
Contributor

palinek commented Mar 27, 2025

Sorry, but I don't see a benefit of having visually separated list of main member. Wouldn't it be enough just to assure, that the main member is always first in the list of members?

@TGODiamond
Copy link
Author

There can be more than one main member in a group.

@palinek
Copy link
Contributor

palinek commented Mar 27, 2025

Sure, and what is the purpose of showing them in GUI separated list?
Again wouldn't it be enough just to put the main members as first in the list of standard members?

@TGODiamond
Copy link
Author

TGODiamond commented Mar 27, 2025

Users can both be a main member and a normal member of a group at the same time.

How should that be shown in the group GUI?

However, I find it understandable to show both normal and main members in the same column in the group list GUI.

@palinek
Copy link
Contributor

palinek commented Mar 27, 2025

How should that be shown in the group GUI?

First in the list with disabled checkbox?

@TGODiamond
Copy link
Author

Then there will be both the main members with checkbox disabled and and normal members with selectable checkboxes, which will ALWAYS include the main members as normal users, since they must also be controlled if they are normal members or not.

This means that there will 2 entries for every member, if they are a main member, which will be confusing, especially for new people to this program.

@TGODiamond
Copy link
Author

I could move the main members list to be over the normal members, and make the main members list thin.

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