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

Solution #379

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Solution #379

wants to merge 12 commits into from

Conversation

markus2812
Copy link

No description provided.

Copy link

@vasylhnatiuk vasylhnatiuk left a comment

Choose a reason for hiding this comment

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

fix tests

cinema/models.py Outdated
class Movie(models.Model):
title = models.CharField(max_length=255)
description = models.TextField()
actors = models.ManyToManyField(Actor)

Choose a reason for hiding this comment

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

add related name

cinema/models.py Outdated
class Movie(models.Model):
title = models.CharField(max_length=255)
description = models.TextField()
actors = models.ManyToManyField(Actor)
genres = models.ManyToManyField(Genre)

Choose a reason for hiding this comment

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

add related name

cinema/views.py Outdated
def patch(self, request, *args, **kwargs):
actor = self.get_object()
serializer = ActorSerializer(actor, data=request.data, partial=True)
if serializer.is_valid():

Choose a reason for hiding this comment

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

use serializer.is_valid(raise_exception=True)

Copy link
Author

Choose a reason for hiding this comment

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

I fixed the bugs, ran git add . and ran git push origin develop

Copy link

@vasylhnatiuk vasylhnatiuk left a comment

Choose a reason for hiding this comment

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

you pushed nothing

@markus2812 markus2812 closed this Nov 2, 2023
@markus2812 markus2812 reopened this Nov 2, 2023
Copy link

@vasylhnatiuk vasylhnatiuk left a comment

Choose a reason for hiding this comment

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

cinema/views.py Outdated
if serializer.is_valid():
def post(self, request):
serializer = GenreSerializer(data=request.data)
if serializer.is_valid(raise_exception=True):

Choose a reason for hiding this comment

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

Suggested change
if serializer.is_valid(raise_exception=True):
serializer.is_valid(raise_exception=True):

redundant if statement

Choose a reason for hiding this comment

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

fix in all places

cinema/urls.py Outdated

urlpatterns = [
path("movies/", movie_list, name="movie-list"),
path("movies/<int:pk>/", movie_detail, name="movie-detail"),
path("", include(router.urls)),

Choose a reason for hiding this comment

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

BTW, no need to include when basepath not provided. Use: urlpatterns = [list of urls] + router.urls

Copy link

@Yurnerroo Yurnerroo left a comment

Choose a reason for hiding this comment

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

fix tests

@markus2812 markus2812 requested a review from Yurnerroo November 3, 2023 11:24
cinema/models.py Outdated
first_name = models.CharField(max_length=255)
last_name = models.CharField(max_length=255)

def __str__(self):

Choose a reason for hiding this comment

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

add return annotations throughout the code

@markus2812 markus2812 requested a review from Yurnerroo November 3, 2023 11:42
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.

4 participants