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

Add User's Group Membership Details to User Info (Fixes #49) #76

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

KimFarida
Copy link
Contributor

Add User Group Membership Details to Fas Info Command

Description:

This pull request modifies the FasHandler class to enhance the user_info command by displaying a user's Fedora Accounts group memberships along with their membership types (member or sponsor).
PR aims to fix #49
Changes:

  • Implemented the get_user_groups function. The modifications improve how the function gathers and presents user group membership information.
    The function no longer relies solely on the initial groups list returned by the API.
    It iterates through each group name and performs an additional get_group_membership call for sponsors specifically.
    • Based on the results of the additional call, it assigns the appropriate membership type ("member" or "sponsor") to each group.
  • The _user_info function is updated to iterate through the user's retrieved groups (a dictionary with group names as keys and membership types as values).
  • For each group, it combines the group name and membership type into a single string using f-strings for clear formatting.
  • These formatted strings are appended to a list, which is then joined with commas and added to the response message as the "Groups :" section.

Benefits:

  • Provides more comprehensive user information by including membership details.
  • Enhances the user experience by offering a clearer picture of a user's involvement in Fedora Accounts groups.

Implementation:

The following code snippet demonstrates the changes made to the _user_info function:

    async def _user_info(self, evt: MessageEvent, username: str | None) -> None:
        await evt.mark_read()
        try:
            user = await get_fasuser(username or evt.sender, evt, self.plugin.fasjsonclient)
            groups = await self.plugin.fasjsonclient.get_user_groups(user.get("username"))
        except InfoGatherError as e:
            await evt.respond(e.message)
            return

        respond_message = (
            f"User: {user.get('username')},{NL}"
            f"Name: {user.get('human_name')},{NL}"
            f"Pronouns: {' or '.join(user.get('pronouns') or ['unset'])},{NL}"
            f"Creation: {user.get('creation')},{NL}"
            f"Timezone: {user.get('timezone')},{NL}"
            f"Locale: {user.get('locale')},{NL}"
            f"GPG Key IDs: {' and '.join(k for k in user['gpgkeyids'] or ['None'])}{NL}"
        )

        if groups:
            respond_message += f"Groups : {', '.join(groups)}{NL}"

        await evt.respond(respond_message)

Implemented get_user_groups function

    async def get_user_groups(self, username, params=None):
        try:
            response = await self._get("/".join(["users", username, "groups"]), params=params)
            user_groups = response.json().get("groups", [])

            group_details = []
            for groupname in user_groups:

                membership_type = await self.get_group_membership(
                    groupname, "sponsors", params=params
                )

                if membership_type is None:
                    membership_type = "member"
                elif username in membership_type:
                    membership_type = "sponsor"

                group_details.append({"groupname": groupname, "membership_type": membership_type})

            return group_details

        except NoResult as e:
            raise InfoGatherError(
                f"Sorry, but Fedora Accounts user '{username}' does not exist"
            ) from e
        except InfoGatherError as e:
            raise

Testing:

  • Unit tests were be updated to verify the correct retrieval and display of user group membership details within the user_info command response.

This pull request aims to improve the user experience of the user_info command by providing more informative group membership details. I appreciate your review and feedback.

Below are the screenshots of testcases passed

Screenshot 2024-03-26 at 00 03 20 Screenshot 2024-03-26 at 00 04 21

KimFarida and others added 7 commits March 25, 2024 01:15
…esolves fedora-infra#51)

This commit updates the help text for the user info and user hello commands

within the FasHandler class. The descriptions are modified to accurately

reflect the information provided by each command:

user info: Returns detailed information about a Fedora user, including username, human name, pronouns, creation date, timezone, locale, and GPG key IDs.

user hello: Returns a brief introduction of a Fedora user, including username, human name, and pronouns (if available).

This clarifies the distinction between the commands and improves the user experience

when interacting with the bot.

Signed-off-by: [KimFarida] <[farimomoh@gmail.com]>
Copy link
Member

@jeremycline jeremycline left a comment

Choose a reason for hiding this comment

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

It looks really good, I think there's one bug from where things weren't indented enough and a few simplifications/style fixes to consider.

fedora/clients/fasjson.py Show resolved Hide resolved
fedora/clients/fasjson.py Outdated Show resolved Hide resolved
fedora/clients/fasjson.py Outdated Show resolved Hide resolved
fedora/clients/fasjson.py Outdated Show resolved Hide resolved
fedora/clients/fasjson.py Outdated Show resolved Hide resolved
fedora/fas.py Outdated Show resolved Hide resolved
tests/clients/test_fasjson.py Outdated Show resolved Hide resolved
@KimFarida
Copy link
Contributor Author

@jeremycline @ryanlerch I think we're all good to go now!
Cleaning this up was a wonderful learning experience

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.

add group membership to !user info
2 participants