-
Notifications
You must be signed in to change notification settings - Fork 16
Refactor container creation to use context and saver #1031
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
Conversation
Updated ContainerBuilder's firstOrCreate method to accept a web.EventContext and use the model's Saver for object creation, ensuring proper context and transaction handling. Added ParamContainerCreate constant. Updated dependencies in gentool/go.mod and go.sum.
Here's the code health analysis summary for commits Analysis Summary
|
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors container creation to use web context and model savers instead of direct database transactions. The changes improve transaction handling by propagating database context through the web framework and using model-specific savers for object creation.
- Updates ContainerBuilder's firstOrCreate method to accept web.EventContext and use model savers
- Adds ParamContainerCreate constant for tracking container creation requests
- Updates indirect dependencies in gentool module
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
utils/testflow/gentool/go.mod | Updates indirect dependency versions for testing libraries |
pagebuilder/editor.go | Adds ParamContainerCreate constant |
pagebuilder/builder.go | Refactors container creation to use context-aware savers and proper transaction handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ctx.R.Form.Set(ParamContainerCreate, "1") | ||
return db.Transaction(func(tx *gorm.DB) (vErr error) { | ||
ctx.WithContextValue(gorm2op.CtxKeyDB{}, tx) | ||
defer ctx.WithContextValue(gorm2op.CtxKeyDB{}, nil) |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context value is being set and then immediately cleared in the defer statement, which means the database transaction will not be available to the saver function. The defer should be moved after the saver calls or the context clearing should be removed.
defer ctx.WithContextValue(gorm2op.CtxKeyDB{}, nil) |
Copilot uses AI. Check for mistakes.
|
||
if len(cons) == 0 { | ||
if vErr = tx.Create(obj).Error; vErr != nil { | ||
if vErr = saver(obj, "", ctx); vErr != nil { |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The empty string parameter passed to saver is unclear. Consider using a named constant or adding a comment to explain what this parameter represents.
Copilot uses AI. Check for mistakes.
Introduces a new i18n message for shared container modification warnings and displays a tooltip when editing shared containers in the page builder. Also refines device toggle logic to check for draft status before triggering events.
Reorganized the tooltip and icon structure for shared containers to improve alignment and layout. The icon and content are now wrapped in a flex container for better visual consistency.
Replaces the use of the isSharedContainer param with the con.Shared field to determine if the container is shared when rendering the editing title tooltip.
* style: format code with Gofumpt This commit fixes the style issues introduced in d910c8f according to the output from Gofumpt. Details: #1031 * Use container Shared field in editing title logic Replaces the use of the isSharedContainer param with the con.Shared field to determine if the container is shared when rendering the editing title tooltip. --------- Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Updated ContainerBuilder's firstOrCreate method to accept a web.EventContext and use the model's Saver for object creation, ensuring proper context and transaction handling. Added ParamContainerCreate constant. Updated dependencies in gentool/go.mod and go.sum.
Note
Use ctx-aware Saver to create demo containers and propagate DB tx via context; add ParamContainerCreate; refresh gentool indirect deps.
ContainerBuilder.firstOrCreate
now acceptsctx
and usesmb.Editing().Creating().Saver
instead oftx.Create
, settinggorm2op.CtxKeyDB
in context and flaggingParamContainerCreate
in the request.Builder.firstOrCreateDemoContainers
passesctx
tocon.firstOrCreate
.ParamContainerCreate
constant ineditor.go
.go-spew
,go-cmp
,go-difflib
,testify
; updatego.sum
.Written by Cursor Bugbot for commit a4ed1f7. This will update automatically on new commits. Configure here.