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

♻️(serializers) move business logic to serializers #414

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

mjeammet
Copy link
Member

@mjeammet mjeammet commented Sep 19, 2024

Purpose

While working on the feature allowing to request a token for another dimail user, I realized business logic probably didn't belong to where we had put it (the model's save method).

The purpose of this PR is to suggest a refacto of these elements.

Proposal

Our resulting discussions aligned with best practices : we'll move all business logic from model to serializer.
all API calls (direct and from front) will keep on triggering
expected 3rd party calls while admin actions will uniquely trigger
modifications in our database.

@mjeammet mjeammet force-pushed the mpj/dimail/request-token-for-other-user branch 2 times, most recently from 8f86910 to c069a08 Compare September 20, 2024 10:07
@mjeammet mjeammet changed the title 👔(dimail) move dimail call to serializers ♻️(serializers) move business logic to serializers Sep 20, 2024
we move all business logic from model to serializer.
all API calls (direct and from front) will keep on triggering
expected 3rd party calls while admin actions will uniquely trigger
modifications in our database.
@mjeammet mjeammet force-pushed the mpj/dimail/request-token-for-other-user branch from c069a08 to 3243335 Compare September 20, 2024 10:09
@mjeammet mjeammet self-assigned this Sep 20, 2024
@mjeammet mjeammet merged commit 55d7e84 into main Sep 20, 2024
14 checks passed
@mjeammet mjeammet deleted the mpj/dimail/request-token-for-other-user branch September 20, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants