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.

Sign up for GitHub

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

readOnly Keyword Has Been Removed from AdminUserAuthServiceImpl and UserAuthServiceImpl Services Because Services Have Insert/Update Transactions #72

Merged
merged 1 commit into from
Jun 10, 2023
Merged

readOnly Keyword Has Been Removed from AdminUserAuthServiceImpl and UserAuthServiceImpl Services Because Services Have Insert/Update Transactions #72

merged 1 commit into from
Jun 10, 2023

Conversation

agitrubard
Copy link
Collaborator

No description provided.

@agitrubard
readOnly Keyword Has Been Removed from AdminUserAuthServiceImpl a…
c039eb7 
…nd `UserAuthServiceImpl` Services Because Services Have Insert/Update Transactions
@agitrubard agitrubard requested a review from emreylmaz June 9, 2023 22:04
@agitrubard agitrubard added the bug Something isn't working label Jun 10, 2023
@agitrubard agitrubard added this to the v1.0.0 milestone Jun 10, 2023
emreylmaz
emreylmaz approved these changes Jun 10, 2023
View reviewed changes
Copy link
Contributor

@emreylmaz emreylmaz left a comment

Choose a reason for hiding this comment

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

LGTM! But in the future, we may consider whether the operations related to the user should be rolled back or not.

Rapter1990
Rapter1990 approved these changes Jun 10, 2023
View reviewed changes
Copy link
Contributor

@Rapter1990 Rapter1990 left a comment

Choose a reason for hiding this comment

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

LGTM.

@agitrubard
Copy link
Collaborator Author

LGTM! But in the future, we may consider whether the operations related to the user should be rolled back or not.

Perhaps the @Transactional annotation may not even be necessary here since there is only one database write operation and the other operations are purely read operations.

@agitrubard agitrubard merged commit eaf0718 into main Jun 10, 2023
@agitrubard agitrubard deleted the bugfix/read-only-keyword branch June 10, 2023 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emreylmaz emreylmaz emreylmaz approved these changes

@Rapter1990 Rapter1990 Rapter1990 approved these changes

@skayikci skayikci Awaiting requested review from skayikci skayikci is a code owner

Assignees
No one assigned
Labels
bug Something isn't working
Projects
No open projects
AYS | Back-End Board
Status: Done
Milestone
v1.0.0.MVP
Development

Successfully merging this pull request may close these issues.
3 participants
@agitrubard @Rapter1990 @emreylmaz