Skip to content

Commit

Permalink
fix: Allow contributors to create sites (#906)
Browse files Browse the repository at this point in the history
* fix: Allow contributors to create sites

* fix: Remove tests testing project settings

* chore: Add comment to project settings

Originally the project settings were supposed to give a normal member
the ability to add sites to project, etc. Since then we've changed the
project member role to viewers and contributors. I'm not removing
these settings here, because it might be used for other things in the
future, but I left a comment for anyone trying to figure this out in
the future.
  • Loading branch information
David Code Howard authored Oct 20, 2023
1 parent a8ae1e3 commit 023d194
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 54 deletions.
2 changes: 1 addition & 1 deletion terraso_backend/apps/graphql/schema/sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def mutate_and_get_payload(cls, root, info, **kwargs):
if adding_to_project:
project = cls.get_or_throw(Project, "project_id", kwargs["project_id"])
if not user.has_perm(Project.get_perm("add_site"), project):
raise cls.not_allowed(MutationTypes.ADD)
raise cls.not_allowed(MutationTypes.CREATE)
kwargs["project"] = project
else:
kwargs["owner"] = info.context.user
Expand Down
2 changes: 2 additions & 0 deletions terraso_backend/apps/project_management/models/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@


class ProjectSettings(BaseModel):
"""NOTE: Theses settings are currently ignored, and might be removed later"""

class Meta(BaseModel.Meta):
abstract = False

Expand Down
4 changes: 1 addition & 3 deletions terraso_backend/apps/project_management/permission_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ def allowed_to_change_project(user, project):

@rules.predicate
def allowed_to_add_site_to_project(user, project):
return project.is_manager(user) or (
project.is_member(user) and project.settings.member_can_add_site_to_project
)
return project.is_manager(user) or project.is_contributor(user)


@rules.predicate
Expand Down
7 changes: 7 additions & 0 deletions terraso_backend/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,10 @@ def project_user(project: Project) -> User:
membership_status=Membership.APPROVED,
)
return user


@pytest.fixture
def project_user_w_role(request, project: Project):
user = mixer.blend(User)
project.add_user_with_role(user, request.param)
return user
53 changes: 3 additions & 50 deletions terraso_backend/tests/graphql/mutations/test_sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ def test_site_creation(client_query, user):
assert log_result.metadata == expected_metadata


def test_site_creation_in_project(client, project_manager, project):
@pytest.mark.parametrize("project_user_w_role", ["manager", "contributor"], indirect=True)
def test_site_creation_in_project(client, project_user_w_role, project):
kwargs = site_creation_keywords()
kwargs["projectId"] = str(project.id)
client.force_login(project_manager)
client.force_login(project_user_w_role)
response = graphql_query(CREATE_SITE_QUERY, variables={"input": kwargs}, client=client)
content = json.loads(response.content)
assert "errors" not in content and "errors" not in content["data"]
Expand Down Expand Up @@ -169,54 +170,6 @@ def test_adding_site_owned_by_user_to_project(client, project, site, project_man
assert site.owned_by(project)


@pytest.mark.parametrize("allow_adding_site", [True, False])
def test_user_can_add_site_to_project_if_project_setting_set(
client, project, project_user, site, allow_adding_site
):
project.settings.member_can_add_site_to_project = allow_adding_site
project.settings.save()
project.save()
site.add_owner(project_user)
client.force_login(project_user)
response = graphql_query(
UPDATE_SITE_QUERY,
variables={"input": {"id": str(site.id), "projectId": str(project.id)}},
client=client,
)
content = json.loads(response.content)
if allow_adding_site:
assert content["data"]["updateSite"]["errors"] is None
payload = content["data"]["updateSite"]["site"]
assert payload["id"] == str(site.id)
site.refresh_from_db()
assert site.owned_by(project)
else:
error_result = response.json()["data"]["updateSite"]["errors"][0]["message"]
json_error = json.loads(error_result)
assert json_error[0]["code"] == "update_not_allowed"


@pytest.mark.parametrize("allow_adding_site", [True, False])
def test_user_can_add_new_site_to_project_if_project_setting_set(
client, project, project_user, allow_adding_site
):
project.settings.member_can_add_site_to_project = allow_adding_site
project.settings.save()
project.save()
client.force_login(project_user)
kwargs = site_creation_keywords()
kwargs["projectId"] = str(project.id)
response = graphql_query(CREATE_SITE_QUERY, variables={"input": kwargs}, client=client)
content = json.loads(response.content)
if allow_adding_site:
assert "errors" not in content and "errors" not in content["data"]
payload = content["data"]["addSite"]["site"]
site = Site.objects.get(id=payload["id"])
assert site.owned_by(project)
else:
assert "errors" in content or "errors" in content["data"]


DELETE_SITE_QUERY = """
mutation SiteDeleteMutation($input: SiteDeleteMutationInput!) {
deleteSite(input: $input) {
Expand Down

0 comments on commit 023d194

Please sign in to comment.