Avoid patch on `RelationshipView` deleting relationship instance when constraint would allow null #499

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

Jump to bottom

Merged

sliverc merged 2 commits into django-json-api:master from Alig1493:relationship_view_patch

Oct 25, 2018

Merged

Avoid patch on `RelationshipView` deleting relationship instance when constraint would allow null #499

sliverc merged 2 commits into django-json-api:master from Alig1493:relationship_view_patch

Oct 25, 2018

Conversation

Alig1493

Copy link

Contributor

@Alig1493 Alig1493 commented Oct 22, 2018 •

edited by sliverc

Loading

Fixes #242

Description of the Change

Added test to check for relationship view relationship types and patch accordingly and updated relationship functionality for related fields

Checklist

PR only contains one change (considered splitting up PR)
unit-test added
documentation updated
changelog entry added to CHANGELOG.md
author name in AUTHORS

@Alig1493

Copy link

Contributor Author

Alig1493 commented Oct 22, 2018 •

edited

Loading

@sliverc An attempt to resolve the issue #242.

@Alig1493 Alig1493 changed the title ~~(削除) Added test to check for relationship view relationship types and patc... (削除ここまで)~~ (追記) Patch Relationship View (追記ここまで)

Oct 22, 2018

@Alig1493 Alig1493 changed the title ~~(削除) Patch Relationship View (削除ここまで)~~ (追記) Relationship View Patch (追記ここまで)

Oct 22, 2018

@Alig1493 Alig1493 force-pushed the relationship_view_patch branch 2 times, most recently from 187057f to e3c66b0 Compare

October 22, 2018 14:01

sliverc

sliverc requested changes

Oct 23, 2018

View reviewed changes

Copy link

Member

@sliverc sliverc left a comment

Choose a reason for hiding this comment

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

Nice work! This looks good to me.

See inline comments and also a CHANGELOG entry needs to be added.

rest_framework_json_api/views.py Outdated Show resolved Hide resolved

example/tests/test_views.py Outdated Show resolved Hide resolved

@Alig1493 Alig1493 force-pushed the relationship_view_patch branch from 1fe5206 to 575742b Compare

October 24, 2018 10:15

@Alig1493

Copy link

Contributor Author

Alig1493 commented Oct 24, 2018

@sliverc I applied the suggested changes.

@Alig1493 Alig1493 force-pushed the relationship_view_patch branch from 575742b to 6cb56d3 Compare

October 24, 2018 11:06

sliverc

sliverc requested changes

Oct 24, 2018

View reviewed changes

Copy link

Member

@sliverc sliverc left a comment

Choose a reason for hiding this comment

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

Two linting issues (see inline comments) which concern this PR.

There are other linting issues but that's because deps are not pinned. Needs to be fixed in another PR (will see that I get to it)

example/tests/test_views.py Outdated

'id': str(self.second_comment.id)

}

'links': {'self': 'http://testserver/authors/{}/relationships/comment_set'.format(self.author.id)}

Copy link

Member

@sliverc sliverc Oct 24, 2018

Choose a reason for hiding this comment

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

./example/tests/test_views.py:246:101: E501 line too long (110 > 100 characters)

example/tests/test_views.py Outdated

'id': str(comment.id)

}

'links': {'self': 'http://testserver/authors/{}/relationships/comment_set'.format(self.author.id)}

Copy link

Member

@sliverc sliverc Oct 24, 2018

Choose a reason for hiding this comment

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

/example/tests/test_views.py:259:101: E501 line too long (110 > 100 characters)

@sliverc

Copy link

Member

sliverc commented Oct 25, 2018

master is now fixed after merging #501 so a rebase to master will fix the other linting issues.

@Alig1493 Alig1493 force-pushed the relationship_view_patch branch from a125db9 to 31967bb Compare

October 25, 2018 10:33

@Alig1493

Copy link

Contributor Author

Alig1493 commented Oct 25, 2018

@sliverc I have rebased and pushed.

@sliverc sliverc changed the title ~~(削除) Relationship View Patch (削除ここまで)~~ (追記) Avoid patch on RelationshipView deleting relationship instance when constraint would allow null (追記ここまで)

Oct 25, 2018

@sliverc sliverc force-pushed the relationship_view_patch branch from 31967bb to bbdaa8c Compare

October 25, 2018 14:33

@Alig1493


 Added test to check for relationship view relationship types and patc...

dfaa30d

...h accordingly and updated relationship functionality for related fields

@sliverc sliverc force-pushed the relationship_view_patch branch 2 times, most recently from 368abde to ea6cbfc Compare

October 25, 2018 14:45

sliverc

sliverc approved these changes

Oct 25, 2018

View reviewed changes


 Add changelog entry for patch RelationshipView patch

46a386e

@sliverc sliverc force-pushed the relationship_view_patch branch from ea6cbfc to 46a386e Compare

October 25, 2018 15:16

@sliverc sliverc merged commit b3eed32 into django-json-api:master

Oct 25, 2018

@sliverc

Copy link

Member

sliverc commented Oct 25, 2018

@Alig1493 Thanks. Added missing changelog entry and merged.

Labels

None yet

2 participants

@Alig1493 @sliverc

Avoid patch on RelationshipView deleting relationship instance when constraint would allow null #499

Avoid patch on RelationshipView deleting relationship instance when constraint would allow null #499

Uh oh!

Conversation

@Alig1493 Alig1493 commented Oct 22, 2018 • edited by sliverc Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the Change

Checklist

Uh oh!

Alig1493 commented Oct 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@sliverc sliverc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alig1493 commented Oct 24, 2018

Uh oh!

@sliverc sliverc left a comment

Choose a reason for hiding this comment

Uh oh!

@sliverc sliverc Oct 24, 2018

Choose a reason for hiding this comment

Uh oh!

@sliverc sliverc Oct 24, 2018

Choose a reason for hiding this comment

Uh oh!

sliverc commented Oct 25, 2018

Uh oh!

Alig1493 commented Oct 25, 2018

Uh oh!

sliverc commented Oct 25, 2018

Uh oh!

Uh oh!

Avoid patch on `RelationshipView` deleting relationship instance when constraint would allow null #499

Avoid patch on `RelationshipView` deleting relationship instance when constraint would allow null #499

@Alig1493 Alig1493 commented Oct 22, 2018 •

edited by sliverc

Loading

Alig1493 commented Oct 22, 2018 •

edited

Loading