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

Refactored base-email-view #358

Closed

Conversation

MihirKohli
Copy link

Resolved #354

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@MihirKohli please check the details of the build / QA check failures.

user.first().is_staff is True
and not qs.filter(pk=self.request.user.id).exists()
user.first().is_staff is True
and not qs.filter(pk=self.request.user.id).exists()
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 not be changed

def get_serializer_context(self):
if getattr(self, 'swagger_fake_view', False):
# To get rid of assertion error raised in
# the dev server, and for schema generation
Copy link
Member

Choose a reason for hiding this comment

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

do not remove these comments please

@MihirKohli
Copy link
Author

made changes do check it

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

made changes do check it

Please change the commit message to:

[refactor] Refactor BaseEmailView to use the FilterByParent mixin #354

Closes #354

@@ -198,7 +199,7 @@ def update(self, request, *args, **kwargs):
)


class BaseEmailView(ProtectedAPIMixin, GenericAPIView):
class BaseEmailView(ProtectedAPIMixin, FilterByParent, GenericAPIView):
Copy link
Member

Choose a reason for hiding this comment

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

Yeah ok, we're inheriting from this FilterByParent now but there's some code here in BaseEmailView which is now redundant and has to be removed, which is not being done in this patch, so the goal of the issue is not being reached yet.

@pandafy
Copy link
Member

pandafy commented Feb 19, 2024

@MihirKohli I am closing this PR due to inactivity. Feel free to re-open the PR if you want to continue with the requested changes.

@pandafy pandafy closed this Feb 19, 2024
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.

[refactor] Refactor BaseEmailView to use the FilterByParent mixin
3 participants