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

21429 - staff account search for members #3169

Merged
merged 3 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions auth-api/src/auth_api/models/dataclass.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ class OrgSearch: # pylint: disable=too-many-instance-attributes
id: str
decision_made_by: str
org_type: str
include_members: bool
member_search_text: str
page: int
limit: int

Expand Down
16 changes: 16 additions & 0 deletions auth-api/src/auth_api/models/org.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,22 @@ def search_org(cls, search: OrgSearch, environment: str):
query = query.filter(Org.branch_name.ilike(f"%{search.branch_name}%"))
if search.name:
query = query.filter(Org.name.ilike(f"%{search.name}%"))
if search.member_search_text:
member_exists_subquery = text(
"""
EXISTS (
SELECT 1
FROM memberships
JOIN users ON users.id = memberships.user_id
WHERE memberships.org_id = orgs.id
AND memberships.status = 1
AND users.status = 1
AND CONCAT(users.last_name, ' ', users.first_name, ' ', users.username) ILIKE :member_search_text
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to use string subquery due to model cyclic dependencies.
Membership model has an org import.

Copy link
Collaborator

@seeker25 seeker25 Dec 5, 2024

Choose a reason for hiding this comment

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

Haha, this should be refactored to a service preferably and done in ORM.
I see it uses contacts and contact links which have nothing to do with orgs (in the model :( , not your changes .. )

The only spot it's called is from the service too. Service already is 1000 lines, looks crowded too.

image

If this was pay, would definitely have to refactor.

Copy link
Collaborator

@seeker25 seeker25 Dec 5, 2024

Choose a reason for hiding this comment

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

As long as there is a unit test should be ok (if the datamodel changes etc), but I'll leave it up to you to decide

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 we should make a ticket for the sbc-connect team to break alot of these dependencies as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I started looking at doing a refactor, and it became a larger undertaking then I wanted for the scope of this ticket. Since it was just a subquery I kept it simple.

I agree we should probably have some kind of tech debt ticket to clean this up.

"""
).params(member_search_text=f"%{search.member_search_text}%")

query = query.filter(member_exists_subquery)

query = cls._search_by_business_identifier(query, search.business_identifier, environment)
query = cls._search_for_statuses(query, search.statuses)
Expand Down
2 changes: 2 additions & 0 deletions auth-api/src/auth_api/resources/v1/org.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ def search_organizations():
extract_numbers(request.args.get("id", None)),
request.args.get("decisionMadeBy", None),
request.args.get("orgType", None),
bool(request.args.get("includeMembers", False)),
request.args.get("members", None),
ochiu marked this conversation as resolved.
Show resolved Hide resolved
int(request.args.get("page", 1)),
int(request.args.get("limit", 10)),
)
Expand Down
7 changes: 6 additions & 1 deletion auth-api/src/auth_api/services/org.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from auth_api.models.affidavit import Affidavit as AffidavitModel
from auth_api.models.dataclass import Activity, DeleteAffiliationRequest
from auth_api.models.org import OrgSearch
from auth_api.schemas import ContactSchema, InvitationSchema, OrgSchema
from auth_api.schemas import ContactSchema, InvitationSchema, MembershipSchema, OrgSchema
from auth_api.services.user import User as UserService
from auth_api.services.validators.access_type import validate as access_type_validate
from auth_api.services.validators.account_limit import validate as account_limit_validate
Expand Down Expand Up @@ -800,6 +800,11 @@ def search_orgs(search: OrgSearch, environment): # pylint: disable=too-many-loc
if include_invitations and org.invitations
else []
),
"members": (
MembershipSchema(exclude=("org", "user.contacts")).dump(org.members, many=True)
ochiu marked this conversation as resolved.
Show resolved Hide resolved
if search.include_members and org.members
else []
),
}
)
return orgs_result
Expand Down
48 changes: 48 additions & 0 deletions auth-api/tests/unit/api/test_org.py
Original file line number Diff line number Diff line change
Expand Up @@ -2932,3 +2932,51 @@ def test_update_org_api_access(client, jwt, session, keycloak_mock): # pylint:d
assert rv.status_code == HTTPStatus.OK
dictionary = json.loads(rv.data)
assert dictionary["hasApiAccess"] is True


def test_search_org_members(client, jwt, session, keycloak_mock): # pylint:disable=unused-argument
"""Assert that a list of members for an org search can be retrieved."""
user_info = TestJwtClaims.public_user_role
headers = factory_auth_header(jwt=jwt, claims=user_info)
client.post("/api/v1/users", headers=headers, content_type="application/json")
client.post("/api/v1/orgs", data=json.dumps(TestOrgInfo.org1), headers=headers, content_type="application/json")

headers = factory_auth_header(jwt=jwt, claims=TestJwtClaims.staff_view_accounts_role)
rv = client.get(
f"/api/v1/orgs?status=ACTIVE&includeMembers=true&members={user_info['preferred_username']}",
headers=headers,
content_type="application/json",
)
assert rv.status_code == HTTPStatus.OK
dictionary = json.loads(rv.data)
assert dictionary["orgs"]
assert len(dictionary["orgs"][0]["members"]) == 1
member = dictionary["orgs"][0]["members"][0]
assert member["membershipTypeCode"] == "ADMIN"
assert member["user"]
user = member["user"]
assert user["username"] == user_info["preferred_username"]

rv = client.get(
f"/api/v1/orgs?status=ACTIVE&includeMembers=true&members={user_info['lastname']} {user_info['firstname']}",
headers=headers,
content_type="application/json",
)

dictionary = json.loads(rv.data)
assert dictionary["orgs"]
assert len(dictionary["orgs"][0]["members"]) == 1
member = dictionary["orgs"][0]["members"][0]
assert member["membershipTypeCode"] == "ADMIN"
assert member["user"]
user = member["user"]
assert user["firstname"] == user_info["firstname"]
assert user["lastname"] == user_info["lastname"]

rv = client.get(
"/api/v1/orgs?status=ACTIVE&includeMembers=true&members=NOTHING",
headers=headers,
content_type="application/json",
)
dictionary = json.loads(rv.data)
assert not dictionary["orgs"]
Loading