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

Refactor les transactions pour pas raiser dedans #4605

Merged
merged 1 commit into from
Jan 26, 2018

Conversation

vhf
Copy link
Contributor

@vhf vhf commented Sep 1, 2017

Q R
Type de modification refacto
Ticket(s) (issue(s)) concerné(s) -

Backporté depuis #4389

QA

  • Instruction 1 (exemple : lancez python manage.py migrate et yarn test)
  • Instruction 2
  • ...
  • Code review

is_visible = self.object.is_visible
if can_read and ((is_author and is_visible) or request.user.has_perm('forum.change_post')):
return super(PostEdit, self).dispatch(request, *args, **kwargs)
raise PermissionDenied
Copy link
Member

Choose a reason for hiding this comment

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

ça vaudrait presque le coup d'avoir notre propre context_manager qui raise PermissionDenied si on n'a pas fait de return à la fin du with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C'est une idée. Bon là c'est explicite et simple, c'est pas plus mal.

@@ -117,6 +115,7 @@ def send_email(self, notification):
try:
msg.send()
except SMTPException:
LOG.error('Failed sending mail for %s', self, exc_info=True)
pass
Copy link
Member

Choose a reason for hiding this comment

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

du coup le pass est inutile.

@vhf vhf force-pushed the refactor-transactions branch 2 times, most recently from 58c7824 to fe1f7f4 Compare September 1, 2017 12:49
@zestedesavoir zestedesavoir deleted a comment from coveralls Sep 1, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Sep 1, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Sep 1, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Sep 1, 2017
@coveralls
Copy link

coveralls commented Sep 1, 2017

Coverage Status

Coverage increased (+0.02%) to 89.678% when pulling 70dbcd1 on vhf:refactor-transactions into c6e959e on zestedesavoir:dev.

@zestedesavoir zestedesavoir deleted a comment from coveralls Sep 1, 2017
@motet-a
Copy link
Contributor

motet-a commented Sep 18, 2017

@artragis
Copy link
Member

Apparament pas de conflit insoluble. Néanmoins un rebase est nécessaire pour faire passer travis sur la nouvelle base de code.

@motet-a motet-a added the C-Back Concerne le back-end Django label Sep 19, 2017
@artragis
Copy link
Member

tu peux faire un petit rebase stp?

@artragis
Copy link
Member

tu pourras rebase ça @vhf ?

@vhf
Copy link
Contributor Author

vhf commented Jan 26, 2018

@artragis C'est fait.

@artragis artragis merged commit 26eff53 into zestedesavoir:dev Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants