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

Project creation and update API #1034

Closed

Conversation

mldulaney
Copy link
Contributor

@mldulaney mldulaney commented Apr 19, 2019

See #1035

@mldulaney mldulaney requested a review from madprime April 19, 2019 22:50
Signed-off-by: Mairi Dulaney <[email protected]>
Signed-off-by: Mairi Dulaney <[email protected]>
Signed-off-by: Mairi Dulaney <[email protected]>
Copy link
Member

@madprime madprime left a comment

Choose a reason for hiding this comment

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

looking good! I noted some issues and questions, and wrote up #1035

Not present in this are behaviors we expect diy experiment projects to have, based on that boolean. that can be a separate PR - I wrote these up as #1036

private_sharing/api_authentication.py Outdated Show resolved Hide resolved
private_sharing/api_authentication.py Outdated Show resolved Hide resolved
private_sharing/api_views.py Outdated Show resolved Hide resolved
private_sharing/api_views.py Outdated Show resolved Hide resolved
private_sharing/forms.py Outdated Show resolved Hide resolved
private_sharing/api_views.py Outdated Show resolved Hide resolved
private_sharing/models.py Outdated Show resolved Hide resolved
private_sharing/models.py Outdated Show resolved Hide resolved
private_sharing/api_views.py Show resolved Hide resolved
Signed-off-by: Mairi Dulaney <[email protected]>
Signed-off-by: Mairi Dulaney <[email protected]>
Signed-off-by: Mairi Dulaney <[email protected]>
@mldulaney mldulaney changed the title WIP -- Initial project creation API Project creation and update API Apr 24, 2019
* commit '921092d9':
  Set queryset order for member-exchange endpoint
Copy link
Member

@madprime madprime left a comment

Choose a reason for hiding this comment

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

Completing in favor of sharing what I have now -- since a major comment here is regarding the validation via serializer rather than form...

is_academic_or_nonprofit: False
add_data: false
explore_share: false
short_description: first 139 chars of long_description plus an elipse
Copy link
Member

Choose a reason for hiding this comment

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

typo: ellipsis


def get_short_description(self, long_description):
"""
Return first 139 chars of long_description plus an elipse.
Copy link
Member

Choose a reason for hiding this comment

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

typo: ellipsis

@@ -13,6 +19,36 @@
UserModel = get_user_model()


def make_oauth2_tokens(project, user):
"""
Returns a tuple, an AccessToken object and a RefreshToken object given a project and a user.
Copy link
Member

Choose a reason for hiding this comment

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

Creates and returns a tuple, AccessToken and RefreshToken, given a project and user.

^^^ this avoids implying a lookup might occur -- it's going to create these.

def make_oauth2_tokens(project, user):
"""
Returns a tuple, an AccessToken object and a RefreshToken object given a project and a user.
:param project: An oath2 project
Copy link
Member

Choose a reason for hiding this comment

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

Typo: OAuth2

@@ -156,3 +160,49 @@ def to_representation(self, obj):
rep.pop("username")

return rep


class ProjectAPISerializer(serializers.Serializer):
Copy link
Member

Choose a reason for hiding this comment

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

OAuth2ProjectSerializer


class ProjectCreateAPIView(APIView):
"""
Create a project via API
Copy link
Member

Choose a reason for hiding this comment

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

Creates an OAuth2DataRequestProject via API.

return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)


class ProjectUpdateAPIView(APIView):
Copy link
Member

Choose a reason for hiding this comment

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

OAuth2ProjectUpdateAPIView


class ProjectUpdateAPIView(APIView):
"""
API Endpoint to update a project.
Copy link
Member

Choose a reason for hiding this comment

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

... to update an OAuth2DataRequestProject

coordinator=member,
leader=member.name,
request_username_access=False,
diyexperiment=True,
Copy link
Member

Choose a reason for hiding this comment

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

this should default to False and be True if the parameter was provided and is true :)

"""
member = get_oauth2_member(request).member
serializer = ProjectAPISerializer(data=request.data)
if serializer.is_valid():
Copy link
Member

Choose a reason for hiding this comment

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

Our other APIs use re-use the on site forms for validation: https://github.com/OpenHumans/open-humans/blob/master/open_humans/api_views.py

I think we should be doing the same here, to have consistency in validation (i.e. re-using OAuth2DataRequestProjectForm)

@mldulaney mldulaney closed this Apr 25, 2019
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.

2 participants