diff --git a/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsEmptyResults.js b/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsEmptyResults.js index 0b5e7e6f8..a658b042a 100644 --- a/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsEmptyResults.js +++ b/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsEmptyResults.js @@ -13,7 +13,7 @@ class MembershipRequestsEmptyResultsCmp extends Component {
- {i18next.t("No matching members found.")} + {i18next.t("No matching membership requests found.")}
{queryString && (

diff --git a/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsResultItem.js b/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsResultItem.js index d1c681079..eca887838 100644 --- a/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsResultItem.js +++ b/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/MembershipRequestsResultItem.js @@ -45,20 +45,12 @@ export class MembershipRequestsResultItem extends Component { membershipRequest: { member }, membershipRequest, } = this.state; - console.log("********************"); - console.log("membershipRequest"); - console.dir(membershipRequest); - console.log("********************"); const request = buildRequest(membershipRequest, ["accept", "decline"]); - console.log("********************"); - console.log("request"); - console.dir(request); - console.log("********************"); - const { api: membershipRequestsApi } = this.context; const roles = rolesCanAssign[member.type]; const expiration = formattedTime(request.expires_at); + return ( @@ -104,9 +96,10 @@ export class MembershipRequestsResultItem extends Component { console.log("actionSuccessCallback called")} + request={request} + actionSuccessCallback={() => { + window.location.reload(); + }} /> diff --git a/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/index.js b/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/index.js index 7eefec3e0..8986ba6de 100644 --- a/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/index.js +++ b/invenio_communities/assets/semantic-ui/js/invenio_communities/members/membership_requests/index.js @@ -39,8 +39,6 @@ const dataAttr = document.getElementById( const community = JSON.parse(dataAttr.community); const communitiesAllRoles = JSON.parse(dataAttr.communitiesAllRoles); const communitiesRolesCanAssign = JSON.parse(dataAttr.communitiesRolesCanAssign); -// TODO: Decision flow: do we need? -// const permissions = JSON.parse(dataAttr.permissions); const appName = "InvenioCommunities.MembershipRequestsSearch"; diff --git a/invenio_communities/members/records/api.py b/invenio_communities/members/records/api.py index 41edcb429..1fb4649b3 100644 --- a/invenio_communities/members/records/api.py +++ b/invenio_communities/members/records/api.py @@ -204,7 +204,10 @@ class Member(Record, MemberMixin): class ArchivedInvitation(Record, MemberMixin): - """An archived invitation record. + """An archived invitation or membership request record. + + The name is a historical legacy. It could be renamed in the future, but we don't + want to rename the index. We are using a record without using the actual JSON document and schema validation normally used in a record. The reason for using a record diff --git a/invenio_communities/members/records/dumpers.py b/invenio_communities/members/records/dumpers.py index 5d797e4b2..8fa476b72 100644 --- a/invenio_communities/members/records/dumpers.py +++ b/invenio_communities/members/records/dumpers.py @@ -30,6 +30,6 @@ def dump(self, record, data): def load(self, data, record_cls): """Load relations.request.type. - TODO: Works without it for now. Potentially revisit? + Works without implementation for now. """ pass diff --git a/invenio_communities/members/resources/config.py b/invenio_communities/members/resources/config.py index 29eb164fa..3ccaac9ad 100644 --- a/invenio_communities/members/resources/config.py +++ b/invenio_communities/members/resources/config.py @@ -46,7 +46,9 @@ class MemberResourceConfig(RecordResourceConfig): AlreadyMemberError: create_error_handler( HTTPJSONException( code=400, - description="A member was already added or invited.", + description=_( + "A membership already exists or is already being assessed." + ), ) ), CommunityDeletedError: create_error_handler( diff --git a/invenio_communities/members/services/config.py b/invenio_communities/members/services/config.py index e7d573b0e..7a62abff0 100644 --- a/invenio_communities/members/services/config.py +++ b/invenio_communities/members/services/config.py @@ -182,6 +182,7 @@ class MemberServiceConfig(RecordServiceConfig, ConfiguratorMixin): search = MemberSearchOptions search_public = PublicSearchOptions search_invitations = InvitationsSearchOptions + search_membership_requests = InvitationsSearchOptions # Use same as invitations links_item = { "actions": LinksForActionsOfMember( diff --git a/invenio_communities/members/services/schemas.py b/invenio_communities/members/services/schemas.py index 471018072..bca6e795c 100644 --- a/invenio_communities/members/services/schemas.py +++ b/invenio_communities/members/services/schemas.py @@ -229,9 +229,24 @@ def get_permissions(self, obj): class MembershipRequestDumpSchema(MemberDumpSchema): - """Schema for dumping membership requests. - - TODO: Decision flow: Investigate if can be merged with InvitationDumpSchema - """ + """Schema for dumping membership requests.""" request = fields.Nested(RequestSchema) + + def get_permissions(self, obj): + """Get permission. + + Only permission to see if current identity can_update role is needed. + + :param obj: api.Member + """ + is_open = obj["request"]["is_open"] + permission_check = self.context["field_permission_check"] + can_update = permission_check( + "members_update", + community_id=obj.community_id, + member=obj, + ) + return { + "can_update_role": is_open and can_update + } diff --git a/invenio_communities/members/services/service.py b/invenio_communities/members/services/service.py index b6fce4ab6..8f00b5bdd 100644 --- a/invenio_communities/members/services/service.py +++ b/invenio_communities/members/services/service.py @@ -87,7 +87,7 @@ def invitation_dump_schema(self): @property def membership_request_dump_schema(self): - """Schema for dumping a membership request to JSON.""" + """Schema for dumping a membership request to REST API JSON.""" return ServiceSchemaWrapper(self, schema=MembershipRequestDumpSchema) # Loading schemas @@ -873,9 +873,8 @@ def search_membership_requests( community_id, "search_membership_requests", self.membership_request_dump_schema, - # Use same as invitations - self.config.search_invitations, # TODO: Decision flow: Rename/merge ? - record_cls=ArchivedInvitation, # TODO: Decision flow: merge or new? + self.config.search_membership_requests, + record_cls=ArchivedInvitation, extra_filter=( dsl.Q("term", **{"active": False}) & dsl.Q( diff --git a/invenio_communities/views/communities.py b/invenio_communities/views/communities.py index e0effcdd4..2f2f909f5 100644 --- a/invenio_communities/views/communities.py +++ b/invenio_communities/views/communities.py @@ -466,7 +466,8 @@ def membership_requests(pid_value, community, community_ui): "invenio_communities/details/members/membership_requests.html", theme=community_ui.get("theme", {}), community=community_ui, - roles_can_assign=_get_roles_can_invite(community.id), # TODO: Decision flow + # We use the same roles as the ones that can be invited + roles_can_assign=_get_roles_can_invite(community.id), permissions=permissions, ) diff --git a/tests/members/test_members_resource.py b/tests/members/test_members_resource.py index 3042e38e4..3b89610fc 100644 --- a/tests/members/test_members_resource.py +++ b/tests/members/test_members_resource.py @@ -238,6 +238,10 @@ def test_search( assert r.status_code == 200 data = r.json assert data["sortBy"] == "name" + expected_links = { + "self": f"https://127.0.0.1:5000/api/communities/{community_id}/members?page=1&size=25&sort=name" # noqa + } + assert expected_links == data["links"] assert data["hits"]["total"] == 3 assert "role" in data["aggregations"] assert "visibility" in data["aggregations"] @@ -300,6 +304,10 @@ def test_search_public( assert r.status_code == 200 data = r.json assert data["sortBy"] == "name" + expected_links = { + "self": f"https://127.0.0.1:5000/api/communities/{community_id}/members?page=1&size=25&sort=name" # noqa + } + assert expected_links == data["links"] assert data["hits"]["total"] == 1 hit = data["hits"]["hits"][0] # Public view has no facets (because that would leak information on @@ -350,9 +358,15 @@ def test_search_invitation( assert hit["permissions"]["can_update_role"] is True assert hit["permissions"]["can_cancel"] is True + request_id = hit["request"]["id"] + expected_links = { + "actions": { + "cancel": f"https://127.0.0.1:5000/api/requests/{request_id}/actions/cancel", + }, + } + assert expected_links == hit["links"] + -# TODO: member serialization/links -# TODO: request serialization/links # TODO: community member can see info # TODO: community non-member can't see info # TODO: facet by role, facet by visibility, define sorts. @@ -375,9 +389,8 @@ def test_post_membership_requests(app, client, headers, community_id, create_use headers=headers, json={"message": "Can I join the club?"}, ) - assert 201 == r.status_code - RequestEvent.index.refresh() + assert 201 == r.status_code # Get links to check url_of_request = r.json["links"]["self"].replace(app.config["SITE_API_URL"], "") @@ -399,22 +412,25 @@ def test_post_membership_requests(app, client, headers, community_id, create_use assert "Can I join the club?" == msg -def test_put_membership_requests( - client, headers, community_id, owner, new_user_data, db -): - # update membership request - # TODO: Implement me! - assert True - - def test_error_handling_for_membership_requests( - client, headers, community_id, owner, new_user_data, db + client, create_user, headers, community_id, owner, new_user_data, db ): - # TODO: Implement me! - # error handling registered - # - permission handling registered - # - duplicate handling registered - assert True + user = create_user({"email": "user_foo@example.org", "username": "user_foo"}) + client = user.login(client) + r = client.post( + f"/communities/{community_id}/membership-requests", + headers=headers, + json={"message": "Can I join the club?"}, + ) + assert 201 == r.status_code + + # Error if duplicate posting + r = client.post( + f"/communities/{community_id}/membership-requests", + headers=headers, + json={"message": "Can I join the club again?"}, + ) + assert 400 == r.status_code def test_get_membership_requests( @@ -451,6 +467,7 @@ def test_get_membership_requests( assert "id" in hit["request"] assert "status" in hit["request"] assert "expires_at" in hit["request"] + # hits > hit > links request_id = hit["request"]["id"] expected_links = { "actions": { @@ -459,11 +476,8 @@ def test_get_membership_requests( }, } assert expected_links == hit["links"] + # hits > hit > permissions + assert hit["permissions"]["can_update_role"] is True + # TODO: Expiration flow : assess if expiration makes sense for membership requests. # assert hit["request"]["expires_at"] is not None - # TODO: Decision flow - # hits > hit > permissions - # assert "permissions" in hit - # assert hit["permissions"]["can_update_role"] is True - # assert hit["permissions"]["can_cancel"] is True - # hits > hit > links