Skip to content

fix: 243 tx to InTx for account,tab,user #258

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

Merged
merged 1 commit into from
Apr 16, 2025
Merged

fix: 243 tx to InTx for account,tab,user #258

merged 1 commit into from
Apr 16, 2025

Conversation

danil-bragin
Copy link

No description provided.

Copy link

cypress bot commented Apr 16, 2025

Default Project    Run #411

Run Properties:  status check passed Passed #411  •  git commit 4970f89d07: Merge pull request #258 from iota-uz/sdk-243
Project Default Project
Branch Review staging
Run status status check passed Passed #411
Run duration 00m 31s
Commit git commit 4970f89d07: Merge pull request #258 from iota-uz/sdk-243
Committer Diyor Khaydarov
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 3
View all changes introduced in this branch ↗︎

@danil-bragin danil-bragin linked an issue Apr 16, 2025 that may be closed by this pull request
@@ -55,7 +55,7 @@ func (c *AccountController) Register(r *mux.Router) {

setRouter := r.PathPrefix(c.basePath).Subrouter()
setRouter.Use(commonMiddleware...)
setRouter.Use(middleware.WithTransaction())
//setRouter.Use(middleware.WithTransaction())
Copy link
Contributor

Choose a reason for hiding this comment

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

You can completely remove this line

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed


var createdUser user.User
err = composables.InTx(ctx, func(txCtx context.Context) error {
data, err = data.SetPassword(data.Password())
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't necessarily need to be part the transaction

}
if err := h.tabService.CreateMany(ctxWithUser, tabs); err != nil {
h.logger.Errorf("Failed to create tab for user %d: %v", user.ID(), err)
err := composables.InTx(ctx, func(txCtx context.Context) error {
Copy link
Contributor

@diyor28 diyor28 Apr 16, 2025

Choose a reason for hiding this comment

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

This call is not necessary since you're opening transactions inside service calls. If you want both calls to be part of a single transaction, create a service method that wraps these two operations in a single transaction

Copy link
Contributor

Choose a reason for hiding this comment

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

made by calling the service

@diyor28
Copy link
Contributor

diyor28 commented Apr 16, 2025

@danil-bragin

@diyor28 diyor28 merged commit 4970f89 into staging Apr 16, 2025
2 checks passed
@diyor28 diyor28 deleted the sdk-243 branch April 16, 2025 15:12
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.

Error Deleting User Tabs: "no database pool found in context"
3 participants