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: changed transaction handling related to Account package #1508

Merged
merged 8 commits into from
Feb 28, 2025

Conversation

kakcy
Copy link
Contributor

@kakcy kakcy commented Feb 6, 2025

This pull request includes significant changes to the pkg/account/api/account.go file, focusing on updating transaction handling and integrating audit log creation. Additionally, it includes corresponding updates to the test file pkg/account/api/account_test.go.

Part of #1474
Part of #1002

@kakcy kakcy force-pushed the update-storage-client-account branch from eac85c2 to 475aec4 Compare February 13, 2025 02:33
@kakcy kakcy marked this pull request as ready for review February 13, 2025 02:53
Comment on lines 232 to 234
return s.auditlogStorage.CreateAuditLog(
contextWithTx,
domainauditlog.NewAuditLog(createAccountEvent, storage.AdminEnvironmentID),
)
Copy link
Contributor Author

@kakcy kakcy Feb 13, 2025

Choose a reason for hiding this comment

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

@cre8ivejp
I tried implementing support for updating AuditLog within the same transaction.
Supporting NoCommand like this will generate an Event, so it would be a good idea to update AuditLog based on that.
Please check when you are free.

@@ -78,6 +78,56 @@ func (h *autoOpsRuleCommandHandler) Handle(ctx context.Context, cmd Command) err
return errUnknownCommand
}

// func (h *autoOpsRuleCommandHandler) NewEvent(ctx context.Context, cmd Command) (*eventproto.Event, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can delete this directly.

@cre8ivejp cre8ivejp removed the request for review from kentakozuka February 17, 2025 04:38
Comment on lines -202 to +203
createAccountEvent, err = domainevent.NewEvent(
createAccountEvent, err = domainevent.NewAdminEvent(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cre8ivejp
Check the code below I think the Account-related AuditLog needs to be saved in AdminAuditLog, so I made some changes.

https://github.com/bucketeer-io/bucketeer/blob/main/pkg/account/command/account_v2.go#L372

@cre8ivejp
Copy link
Member

@kakcy, please fix the conflict.

@kakcy kakcy force-pushed the update-storage-client-account branch from a5573ff to 453174a Compare February 27, 2025 03:05
@kakcy
Copy link
Contributor Author

kakcy commented Feb 27, 2025

@cre8ivejp
I fixed the conflict.

Copy link
Member

@cre8ivejp cre8ivejp left a comment

Choose a reason for hiding this comment

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

Thank you!

@kakcy kakcy merged commit 75f6c44 into main Feb 28, 2025
16 checks passed
@kakcy kakcy deleted the update-storage-client-account branch February 28, 2025 02:11
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.

3 participants