Skip to content

Commit

Permalink
Remove permission for public AF and Changes on mutation
Browse files Browse the repository at this point in the history
Add PR Changes
Fix CI Issue
  • Loading branch information
susilnem committed Aug 12, 2024
1 parent d327d83 commit d4a0456
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 56 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
env:
DOCKER_IMAGE_SERVER: ${{ steps.prep.outputs.tagged_image }}
run: |
docker-compose -f ./gh-docker-compose.yml run --rm server bash -c 'wait-for-it db:5432 && ./manage.py graphql_schema --out /ci-share/schema-latest.graphql' &&
docker compose -f ./gh-docker-compose.yml run --rm server bash -c 'wait-for-it db:5432 && ./manage.py graphql_schema --out /ci-share/schema-latest.graphql' &&
cmp --silent schema.graphql ./ci-share/schema-latest.graphql || {
echo 'The schema.graphql is not up to date with the latest changes. Please update and push latest';
diff schema.graphql ./ci-share/schema-latest.graphql;
Expand All @@ -53,7 +53,7 @@ jobs:
env:
DOCKER_IMAGE_SERVER: ${{ steps.prep.outputs.tagged_image }}
run: |
docker-compose -f ./gh-docker-compose.yml run --rm server bash -c 'wait-for-it db:5432 && ./manage.py makemigrations --check --dry-run' || {
docker compose -f ./gh-docker-compose.yml run --rm server bash -c 'wait-for-it db:5432 && ./manage.py makemigrations --check --dry-run' || {
echo 'There are some changes to be reflected in the migration. Make sure to run makemigrations';
exit 1;
}
Expand All @@ -64,7 +64,7 @@ jobs:
CC_TEST_REPORTER_ID: ${{ secrets.CODE_CLIMATE_ID }}
DOCKER_IMAGE_SERVER: ${{ steps.prep.outputs.tagged_image }}
with:
coverageCommand: docker-compose -f gh-docker-compose.yml run --rm server /code/scripts/run_tests.sh
coverageCommand: docker compose -f gh-docker-compose.yml run --rm server /code/scripts/run_tests.sh
coverageLocations: |
${{github.workspace}}/coverage/coverage.xml:coverage.py
Expand Down
6 changes: 3 additions & 3 deletions .travis-old.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ before_install:

- env > .env
- mv travis-docker-compose.yml docker-compose.yml
- docker-compose pull && docker pull ${DOCKER_IMAGE_SERVER_B} || true
- docker compose pull && docker pull ${DOCKER_IMAGE_SERVER_B} || true
- |
docker build\
--cache-from $(get_image ${DOCKER_IMAGE_SERVER_B} ${DOCKER_IMAGE_SERVER})\
Expand All @@ -24,12 +24,12 @@ before_script:
- /tmp/cc-test-reporter before-build

script:
- docker-compose run server /code/scripts/run_tests.sh
- docker compose run server /code/scripts/run_tests.sh

after_success:
- echo "$DOCKER_PASSWORD" | docker login -u "$DOCKER_USERNAME" --password-stdin
- /tmp/cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT
- docker-compose push server
- docker compose push server
- docker push ${DOCKER_IMAGE_SERVER_B}

deploy:
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ Add this to your `.git/hooks/pre-commit` to generate latest graphql schema befor
echo "pre-commit: Generating graphql schema."
if [ -z `docker ps -q --no-trunc | grep $(docker-compose ps -q web)` ]; then
docker-compose run --rm web ./manage.py graphql_schema --out schema.graphql
docker compose run --rm web ./manage.py graphql_schema --out schema.graphql
else
docker-compose exec -T web ./manage.py graphql_schema --out schema.graphql
docker compose exec -T web ./manage.py graphql_schema --out schema.graphql
fi
```
FYI: If hooks aren't working https://stackoverflow.com/questions/49912695/git-pre-and-post-commit-hooks-not-running
10 changes: 4 additions & 6 deletions apps/analysis_framework/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import copy
from typing import Union
from typing import Union, Optional

from django.db import models
from django.core.exceptions import ValidationError
Expand Down Expand Up @@ -63,15 +63,13 @@ def __init__(self, *args, **kwargs):
def __str__(self):
return self.title

def clone(self, user, overrides={}):
def clone(self, user, title: Optional[str] = None, description: Optional[str] = None):
"""
Clone analysis framework along with all widgets,
filters and exportables
"""
title = overrides.get(
'title', '{} (cloned)'.format(self.title[:230])
) # Strip off extra chars from title
description = overrides.get('description', '')
title = title or "{} (cloned)".format(self.title[:230]) # Strip off extra chars from title
description = description or ""
clone_analysis_framework = AnalysisFramework(
title=title,
description=description,
Expand Down
9 changes: 6 additions & 3 deletions apps/analysis_framework/mutation.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,16 @@ def get_valid_delete_items(cls, info, delete_ids):
)


class CloneAnalysisFramework(AfGrapheneMutation):
class CloneAnalysisFramework(GrapheneMutation):
class Arguments:
data = AnalysisFrameworkCloneInputType(required=True)

result = graphene.Field(AnalysisFrameworkDetailType)
serializer_class = AnalysisFrameworkCloneSerializer
permissions = [AfP.Permission.CAN_CLONE_FRAMEWORK]

@classmethod
def check_permissions(cls, info, **_):
return True # NOTE: Checking permissions in serializer


class AnalysisFrameworkMutationType(DjangoObjectType):
Expand All @@ -117,7 +120,6 @@ class AnalysisFrameworkMutationType(DjangoObjectType):
"""
analysis_framework_update = UpdateAnalysisFramework.Field()
analysis_framework_membership_bulk = BulkUpdateAnalysisFrameworkMembership.Field()
analysis_framework_clone = CloneAnalysisFramework.Field()

class Meta:
model = AnalysisFramework
Expand All @@ -136,4 +138,5 @@ def get_custom_node(_, info, id):

class Mutation(object):
analysis_framework_create = CreateAnalysisFramework.Field()
analysis_framework_clone = CloneAnalysisFramework.Field()
analysis_framework = DjangoObjectField(AnalysisFrameworkMutationType)
25 changes: 23 additions & 2 deletions apps/analysis_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,18 +712,39 @@ def create(self, validated_data):


class AnalysisFrameworkCloneSerializer(serializers.Serializer):
af_id = IntegerIDField(required=True)
title = serializers.CharField(required=True)
description = serializers.CharField(required=False)
project = serializers.PrimaryKeyRelatedField(queryset=Project.objects.all(), required=False)

def validate_af_id(self, af_id):
af = (
AnalysisFramework
.get_for_gq(self.context['request'].user, only_member=False)
.filter(pk=af_id)
.first()
)
if af is None:
raise serializers.ValidationError('Analysis Framework does not exist')
if not af.can_clone(self.context['request'].user):
raise serializers.ValidationError(
'User does not have permission to modify the AF'
)
return af

def validate_project(self, project):
if not project.can_modify(self.context['request'].user):
raise serializers.ValidationError('User does not have permission to modify the project')
return project

def create(self, validated_data):
af = self.context['request'].active_af
new_af = af.clone(self.context['request'].user, validated_data)
# NOTE: af is an obj, check validate_af_id
af = validated_data['af_id']
new_af = af.clone(
self.context['request'].user,
title=validated_data['title'],
description=validated_data.get('description')
)
new_af.add_member(self.context['request'].user, new_af.get_or_create_owner_role())

if project := validated_data.get('project'):
Expand Down
63 changes: 28 additions & 35 deletions apps/analysis_framework/tests/test_mutations.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,18 +786,16 @@ def _query_check(**kwargs):

def test_analysis_framework_clone(self):
query = '''
mutation MyMutation ($id: ID!, $input: AnalysisFrameworkCloneInputType!) {
mutation MyMutation ($input: AnalysisFrameworkCloneInputType!) {
__typename
analysisFramework (id: $id ) {
analysisFrameworkClone(data: $input) {
ok
errors
result {
id
title
description
clonedFrom
}
analysisFrameworkClone(data: $input) {
ok
errors
result {
id
title
description
clonedFrom
}
}
}
Expand All @@ -815,65 +813,60 @@ def test_analysis_framework_clone(self):
private_af = AnalysisFrameworkFactory.create(created_by=member_user, title='AF Private Orginal', is_private=True)
private_af.add_member(low_permission_user)

minput = dict(
public_minput = dict(
title='AF (TEST)',
description='Af description',
afId=af.id,
)
private_minput = dict(
title='AF (TEST)',
description='Af description',
afId=private_af.id,
)

def _public_af_query_check(**kwargs):
return self.query_check(
query,
minput=minput,
mnested=['analysisFramework'],
variables={'id': af.id},
minput=public_minput,
**kwargs,
)

def _private_af_query_check(**kwargs):
return self.query_check(
query,
minput=minput,
mnested=['analysisFramework'],
variables={'id': private_af.id},
minput=private_minput,
**kwargs,
)

# ---------- Without login
_public_af_query_check(assert_for_error=True)
# ---------- With login
self.force_login(non_member_user)
# ---------- Let's Clone a new AF (Using create test data)
_public_af_query_check(assert_for_error=True)

# ---------- With login (with access member)
self.force_login(member_user)
# ---------- With login (with non access member) on public AF
self.force_login(non_member_user)
response = _public_af_query_check()
self.assertEqual(response['data']['analysisFramework']['analysisFrameworkClone']['result']['clonedFrom'], str(af.id))
self.assertEqual(response['data']['analysisFrameworkClone']['result']['clonedFrom'], str(af.id))

# adding project to the input
minput['project'] = project.id
# adding project to the inputs
public_minput['project'] = project.id
private_minput['project'] = project.id

# with Login (non project member)
self.force_login(non_member_user)
_public_af_query_check(assert_for_error=True)

# with Login (project member with no permission on AF)
self.force_login(low_permission_user)
_public_af_query_check(assert_for_error=True)
_public_af_query_check(okay=False)

# with Login (project member with no permission on Private AF)
self.force_login(member_user)
_private_af_query_check(assert_for_error=True)
_private_af_query_check(okay=False)

# With Login (project member with permission on Public AF)
self.force_login(member_user)
response = _public_af_query_check()['data']['analysisFramework']
response = _public_af_query_check()['data']
project.refresh_from_db()
self.assertEqual(str(project.analysis_framework_id), response['analysisFrameworkClone']['result']['id'])

# with Login (project member with permission on Private AF)
private_af.add_member(member_user, role=self.af_owner)
response = _private_af_query_check()['data']['analysisFramework']
response = _private_af_query_check()['data']
project.refresh_from_db()
self.assertEqual(str(project.analysis_framework_id), response['analysisFrameworkClone']['result']['id'])

Expand Down
3 changes: 2 additions & 1 deletion apps/analysis_framework/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ def post(self, request, af_id, version=None):

new_af = analysis_framework.clone(
request.user,
request.data or {},
title=cloned_title,
description=request.data.get('description'),
)
# Set the requesting user as owner member, don't create other memberships of old framework
new_af.add_member(request.user, new_af.get_or_create_owner_role())
Expand Down
3 changes: 2 additions & 1 deletion schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type AnalysisAutomaticSummaryType {
}

input AnalysisFrameworkCloneInputType {
afId: ID!
title: String!
description: String
project: ID
Expand Down Expand Up @@ -148,7 +149,6 @@ type AnalysisFrameworkMutationType {
title: String!
analysisFrameworkUpdate(data: AnalysisFrameworkInputType!): UpdateAnalysisFramework
analysisFrameworkMembershipBulk(deleteIds: [ID!], items: [BulkAnalysisFrameworkMembershipInputType!]): BulkUpdateAnalysisFrameworkMembership
analysisFrameworkClone(data: AnalysisFrameworkCloneInputType!): CloneAnalysisFramework
}

enum AnalysisFrameworkPermission {
Expand Down Expand Up @@ -5066,6 +5066,7 @@ type Mutation {
updateMe(data: UserMeInputType!): UpdateMe
deleteUser: UserDelete
analysisFrameworkCreate(data: AnalysisFrameworkInputType!): CreateAnalysisFramework
analysisFrameworkClone(data: AnalysisFrameworkCloneInputType!): CloneAnalysisFramework
analysisFramework(id: ID!): AnalysisFrameworkMutationType
}

Expand Down

0 comments on commit d4a0456

Please sign in to comment.