Events API #628 #760

josueemartinezz · 2024-03-13T19:43:08Z

Contributor checklist

This pull request is on a separate branch and not the main branch

Description

Added (list, create, retrieve, update, partial_update, destroy) HTTP methods for Events. Ensured event creators are admin associated within their organization.
Modified serializers to validate non-trivial input. Trivial validation checks are removed as they're ensured by the model setup.
Added missing fields for the Event model (according to Figma designs)

TESTING

Extensive testing was done on the backend to ensure the model catches bad user input to eliminate validation checks done in serializers.
Verified that upon organization creation, an organization admin member is allowed to create an event whereas if is_admin is false, HTTP responds appropriately.

NOTE:

breaks might occur as during the testing stage I would receive independent errors coming from authentication and organization models. However, @andrewtavis confirmed to leave these model definitions as they are after manipulating them.

Related issue

Events API #628

github-actions · 2024-03-13T19:43:21Z

Thank you for the pull request!

The activist team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. It'd be great to have you!

Maintainer checklist

The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution
- The contributor's name and icon in remote commits should be the same as what appears in the PR
- If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local activist repo
The TypeScript and formatting workflows within the PR checks do not indicate new errors in the files changed
The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

netlify · 2024-03-13T19:43:22Z

✅ Deploy Preview for activist-org ready!

Name	Link
🔨 Latest commit	`b513a1e`
🔍 Latest deploy log	https://app.netlify.com/sites/activist-org/deploys/65ff067cb99a090007c86753
😎 Deploy Preview	https://deploy-preview-760--activist-org.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

andrewtavis · 2024-03-14T21:58:55Z

Thanks for the hard work here, @josueemartinezz! As an initial check on all of this, would you want to go through the backend check and send along fixes for all of that? @to-sta and I can then take a look afterwards :)

andrewtavis · 2024-03-16T18:50:21Z

Two minor things that I just shared with another contributor that would be helpful for you, @josueemartinezz:

It'd be good if you'd install the Ruff extension for VS Code that will take care of the formatting and import ordering errors :)
Could you go through the Pull Requests section of the contributing guide and look into installing pre-commit as it's described there

Want to stress that there's no issue on our end with the checks failing! All this is very appreciated, and doing the steps above will just take it all to the next level 🚀

to-sta · 2024-03-17T13:59:16Z

backend/entities/views.py

@@ -43,6 +43,11 @@ class OrganizationViewSet(viewsets.ModelViewSet[Organization]):
    serializer_class = OrganizationSerializer
    pagination_class = CustomPagination
    throttle_classes = [AnonRateThrottle, UserRateThrottle]
+
+    def list(self, request: Request, *args: str, **kwagrs: int) -> Response:
+        queryset = Organization.objects.all()


@josueemartinezz

The variable queryset is unused and redundant 😅.

queryset = Event.objects.all() serializer = self.get_serializer(self.queryset, many=True)

to-sta · 2024-03-17T14:02:22Z

backend/entities/views.py

@@ -70,7 +75,7 @@ def destroy(self, request: Request, *args: str, **kwargs: int) -> Response:
        instance = get_object_or_404(Organization, pk=kwargs["pk"])
        instance.delete()
        data = {"message": f'Organization {kwargs["pk"]} has been deleted successfully'}
-        return Response(data, status=status.HTTP_204_NO_CONTENT)
+        return Response(status=status.HTTP_202_ACCEPTED)


Minor thing here. Accepted is good status for this but we generally use the status.HTTP_201_CREATED in case we create an instance.

to-sta · 2024-03-17T14:04:43Z

backend/events/models.py

@@ -26,11 +26,14 @@
 class Event(CreationDeletionMixin):
    id = models.UUIDField(primary_key=True, default=uuid4, editable=False)
    name = models.CharField(max_length=255)
-    tagline = models.CharField(max_length=255, blank=True)
+    tagline = models.CharField(max_length=255, blank=True, null=True)


In django CharField and TextField are never saved as NULL values, therefore there is no need for null=True.

to-sta · 2024-03-17T14:06:23Z

backend/events/models.py

@@ -41,6 +44,8 @@ class Event(CreationDeletionMixin):
    event_icon = models.OneToOneField(
        "content.Image", on_delete=models.CASCADE, null=True, blank=True
    )
+    creation_date = models.DateTimeField(auto_now_add=True)


creation_date and deletion_date are added by the mixin:

class Event(CreationDeletionMixin): ...

to-sta · 2024-03-17T14:10:20Z

backend/entities/views.py

@@ -70,7 +75,7 @@ def destroy(self, request: Request, *args: str, **kwargs: int) -> Response:
        instance = get_object_or_404(Organization, pk=kwargs["pk"])
        instance.delete()
        data = {"message": f'Organization {kwargs["pk"]} has been deleted successfully'}


The variable data is created but it is not used. You have to add it to the response.

to-sta · 2024-03-17T14:15:42Z

@josueemartinezz Thanks for contribution 😃. I added a few comments. How did you test the API?

josueemartinezz · 2024-03-17T20:57:29Z

@josueemartinezz Thanks for contribution 😃. I added a few comments. How did you test the API?

Hi! Thank you so much for your review. I'll get these fixed right away. I tested the API by using Swagger-UI for all the HTTP methods, ensuring bad user input passed the Event model's field validation requirements. If it did, like a user's end_time precedes start_time, I raised exceptions in serializers.py. I tested for foreign key existence, but this again was accounted for by the key checks done by the field type. Please let me know of any testing strategies tips apart from just playing around with inputs on Swagger-UI.

1. Added (list, create, retrieve, update, partial_update, destroy) HTTP methods 2. Modified serializers to validate non-trivial input from user. Trivial validation is ensured by the model setup. 3. Added missing fields for Event model (according to Figma designs)

1. fixed pre-commit and ruff formatting 2. discarded redundant fields from mixin 3. removed null CharField & TextField 4. fixed request status for Destroy instance. 5. removed unused/redundant variables.

andrewtavis · 2024-04-14T16:28:12Z

Heyo @josueemartinezz! We have the other priority backend PRs merged and will get to this when we can! Sorry for the wait! Map for events has been taking some major strides, so the feature set is definitely there for when this is brought in 😊

andrewtavis requested review from to-sta and andrewtavis March 14, 2024 21:55

to-sta reviewed Mar 17, 2024

View reviewed changes

josueemartinezz added 2 commits March 23, 2024 09:32

fix: Resolved issues and formatting

b513a1e

1. fixed pre-commit and ruff formatting 2. discarded redundant fields from mixin 3. removed null CharField & TextField 4. fixed request status for Destroy instance. 5. removed unused/redundant variables.

josueemartinezz closed this by deleting the head repository May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Events API #628 #760

Events API #628 #760

josueemartinezz commented Mar 13, 2024 •

edited

Loading

github-actions bot commented Mar 13, 2024 •

edited by andrewtavis

Loading

netlify bot commented Mar 13, 2024 •

edited

Loading

andrewtavis commented Mar 14, 2024

andrewtavis commented Mar 16, 2024

to-sta Mar 17, 2024

to-sta Mar 17, 2024

to-sta Mar 17, 2024

to-sta Mar 17, 2024

to-sta Mar 17, 2024

to-sta commented Mar 17, 2024

josueemartinezz commented Mar 17, 2024 •

edited by andrewtavis

Loading

andrewtavis commented Apr 14, 2024

Events API #628 #760

Events API #628 #760

Conversation

josueemartinezz commented Mar 13, 2024 • edited Loading

Contributor checklist

Description

Related issue

github-actions bot commented Mar 13, 2024 • edited by andrewtavis Loading

Thank you for the pull request!

Maintainer checklist

netlify bot commented Mar 13, 2024 • edited Loading

✅ Deploy Preview for activist-org ready!

andrewtavis commented Mar 14, 2024

andrewtavis commented Mar 16, 2024

to-sta Mar 17, 2024

Choose a reason for hiding this comment

to-sta Mar 17, 2024

Choose a reason for hiding this comment

to-sta Mar 17, 2024

Choose a reason for hiding this comment

to-sta Mar 17, 2024

Choose a reason for hiding this comment

to-sta Mar 17, 2024

Choose a reason for hiding this comment

to-sta commented Mar 17, 2024

josueemartinezz commented Mar 17, 2024 • edited by andrewtavis Loading

andrewtavis commented Apr 14, 2024

josueemartinezz commented Mar 13, 2024 •

edited

Loading

github-actions bot commented Mar 13, 2024 •

edited by andrewtavis

Loading

netlify bot commented Mar 13, 2024 •

edited

Loading

josueemartinezz commented Mar 17, 2024 •

edited by andrewtavis

Loading