Skip to content

Conversation

molon
Copy link
Contributor

@molon molon commented Sep 22, 2025

Note

Introduce a full-featured starter module (auth/roles/login/pagebuilder/etc.), add gRPC-aware DataOperator wrapper, update microsite to new archiver API, and make small presets robustness tweaks.

  • Starter (new module starter/):
    • Add configurable admin handler with DI (inject), security middleware, permission setup, i18n, media, l10n, activity logging, SEO, publisher, page builder, and mux wiring.
    • Implement authentication/authorization: local/OAuth (Google/Microsoft/GitHub), sessions, TOTP, role seeding/assignment, profile endpoints, helpers, and test utilities.
  • Presets:
    • presets/data_operator.go: new DataOperatorWithGRPC wrapper injecting i18n metadata and converting gRPC errors (BadRequest/LocalizedMessage) into web.ValidationErrors.
    • editing.go/section.go: surface and handle ValidationErrors explicitly in overlays/messages.
    • listener.go: stabilize NotifRowUpdated to a fixed topic string.
    • listing_compo.go: minor condition simplification for selection hover UI.
    • presets.go: lazy one-time build with sync.Once; safer handler nil handling.
  • Microsite:
    • Update to archiver/v4 API (context-aware Identify, new errors/types) and make file list appends thread-safe during extract/upload.
  • Dependencies:
    • Bump and add packages (e.g., archiver v4 alpha.9, gRPC/genproto, connectrpc, jwt, qor5/x v3.1.x, others) and adjust go.sum accordingly.

Written by Cursor Bugbot for commit 4443a5a. This will update automatically on new commits. Configure here.

@Copilot Copilot AI review requested due to automatic review settings September 22, 2025 09:19
Copy link

@Copilot Copilot AI left a 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 adds a comprehensive starter package for QOR5 admin applications, providing a foundation with authentication, user management, role-based permissions, and localization support.

Key changes include:

  • Complete user management system with OAuth support
  • Role-based access control and permission system
  • Internationalization support for multiple languages
  • Page builder integration and media management

Reviewed Changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
starter/user.go User model builder with admin interface configuration
starter/handler.go Main handler implementation with component initialization
starter/auth.go Authentication system with OAuth providers and session management
starter/model.go User model definition with role relationships
starter/messages.go Internationalization messages for multiple languages
starter/pagebuilder.go Page builder integration with SEO and publishing features
starter/perm.go Permission configuration with role-based access control
starter/middlewares.go HTTP middlewares for security and role loading
starter/provider.go Factory functions for handler setup
starter/testhelper.go Testing utilities for authentication and test setup
presets/section.go Enhanced error handling for validation in save operations
presets/listing_compo.go Code cleanup removing redundant nil check
presets/listener.go Simplified string formatting
presets/data_operator.go New gRPC wrapper for data operations
microsite/microsite.go Updated archiver API usage to v4
go.mod Dependency updates including archiver v4 and new packages

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

ACL: string(types.ObjectCannedACLBucketOwnerFullControl),
Endpoint: a.S3Publish.Endpoint,
})
// TODO: @molon 一定要使用 microsite_utils 吗?
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

TODO comment is written in Chinese. All comments should be in English for consistency.

Suggested change
// TODO: @molon 一定要使用 microsite_utils 吗?
// TODO: @molon Is it necessary to use microsite_utils?

Copilot uses AI. Check for mistakes.

// Configure i18n
a.configureI18n(presetsBuilder)

// TODO: @molon 需要 hook ?
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

TODO comment is written in Chinese. All comments should be in English for consistency.

Suggested change
// TODO: @molon 需要 hook ?
// TODO: @molon Need a hook?

Copilot uses AI. Check for mistakes.

presetsBuilder.Build()

handlerHook := common.ChainHook(
// TODO: @molon 可能要整理一下
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

TODO comment is written in Chinese. All comments should be in English for consistency.

Suggested change
// TODO: @molon 可能要整理一下
// TODO: @molon may need to tidy up

Copilot uses AI. Check for mistakes.

Goth: microsoftonline.New(
authConfig.MicrosoftClientKey,
authConfig.MicrosoftClientSecret,
// TODO: @molon 为什么这里偏偏不用给到 OAuthProviderMicrosoftOnline provider 参数呢?
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

TODO comment is written in Chinese. All comments should be in English for consistency.

Suggested change
// TODO: @molon 为什么这里偏偏不用给到 OAuthProviderMicrosoftOnline provider 参数呢?
// TODO: @molon Why don't we need to provide the OAuthProviderMicrosoftOnline provider parameter here?

Copilot uses AI. Check for mistakes.

Roles string
Users string

// TODO: @iBakuman 有很多是不需要的
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

TODO comment is written in Chinese. All comments should be in English for consistency.

Suggested change
// TODO: @iBakuman 有很多是不需要的
// TODO: @iBakuman Many of these are not needed

Copilot uses AI. Check for mistakes.

if err != nil {
return r, err
}
r.RunScript = fmt.Sprintf(`alert("http://localhost:9500/auth/reset-password?id=%s&token=%s")`, uid, token)
Copy link

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

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

Hardcoded localhost URL is not suitable for production environments. Consider making this configurable through the handler's configuration.

Copilot uses AI. Check for mistakes.

Copy link

deepsource-io bot commented Sep 22, 2025

Here's the code health analysis summary for commits 15291ed..4ebde56. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Go LogoGo❌ Failure
❗ 15 occurences introduced
🎯 3 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

codecov bot commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 68.84817% with 357 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
starter/auth.go 65.80% 74 Missing and 19 partials ⚠️
starter/user.go 72.39% 63 Missing and 27 partials ⚠️
starter/handler.go 66.66% 51 Missing and 14 partials ⚠️
starter/testhelper.go 62.88% 24 Missing and 12 partials ⚠️
presets/data_operator.go 57.81% 19 Missing and 8 partials ⚠️
starter/perm.go 72.97% 9 Missing and 1 partial ⚠️
starter/provider.go 47.36% 8 Missing and 2 partials ⚠️
microsite/microsite.go 0.00% 9 Missing ⚠️
starter/pagebuilder.go 86.27% 4 Missing and 3 partials ⚠️
starter/model.go 64.28% 4 Missing and 1 partial ⚠️
... and 2 more
Files with missing lines Coverage Δ
presets/editing.go 82.37% <100.00%> (+0.12%) ⬆️
presets/listener.go 100.00% <100.00%> (ø)
presets/listing_builder.go 82.48% <100.00%> (+0.16%) ⬆️
presets/presets.go 80.64% <100.00%> (+0.12%) ⬆️
presets/section.go 77.91% <100.00%> (+0.13%) ⬆️
presets/listing_compo.go 80.56% <87.50%> (+0.12%) ⬆️
starter/middlewares.go 84.61% <84.61%> (ø)
starter/model.go 64.28% <64.28%> (ø)
starter/pagebuilder.go 86.27% <86.27%> (ø)
microsite/microsite.go 0.00% <0.00%> (ø)
... and 7 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@molon molon requested a review from Copilot September 23, 2025 13:58
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

presetsBuilder.Build()

handlerHook := common.ChainHook(
// TODO: @molon 可能要整理一下
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

This TODO comment is in Chinese and should be translated to English for consistency with the rest of the codebase.

Suggested change
// TODO: @molon 可能要整理一下
// TODO: @molon may need to tidy up or refactor this

Copilot uses AI. Check for mistakes.

cursor[bot]

This comment was marked as outdated.

Bumped github.com/qor5/x/v3 to v3.1.3-0.20251016020953-4d227d199456 and github.com/theplant/inject to v1.1.0 for improved compatibility and latest features.
Copy link

cursor bot commented Oct 16, 2025

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on November 13.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

zhangshanwen and others added 5 commits October 16, 2025 10:57
Removed old versions of github.com/qor5/x/v3 and updated github.com/theplant/inject to v1.1.0 in go.sum to reflect current dependencies.
Modified calls to s3x.SetupClient in handler.go and pagebuilder.go to pass an additional nil parameter. This aligns with changes in the SetupClient function signature and ensures correct S3 client configuration for media and publishing storage.
Introduced examples for saver validation in detailing and editing presets, including corresponding tests. Added a listing filter notification example and test. Registered new examples in mux. Updated test dependencies in gentool/go.mod and go.sum.
This commit fixes the style issues introduced in ce99c3c according to the output
from Gofumpt.

Details: #1030
Updated the validation logic in PresetsDetailSaverValidation to use GlobalError instead of FieldError for the 'system' name restriction. Adjusted the corresponding test to check for the error in the correct output.
zhangshanwen and others added 4 commits October 16, 2025 15:06
Introduces TestServeHTTP_HandlerNilAfterOnce_Returns500 to verify that ServeHTTP returns a 500 error and logs appropriate messages when the handler is nil after warmupOnce is marked done. This improves coverage for error scenarios in the Builder.
This commit fixes the style issues introduced in 6ebeb52 according to the output
from Gofumpt.

Details: #1030
Introduced an embedded YAML default configuration file and a new InitializeConfig function using the confx package for configuration management. Also added related test files and updated dependencies in go.mod and go.sum.
secret: "test"
baseURL: "http://localhost:10001"
initialUserEmail: "[email protected]"
initialUserPassword: "admin123456789"
Copy link

@aikido-pr-checks aikido-pr-checks bot Oct 17, 2025

Choose a reason for hiding this comment

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

Exposed secret in starter/embed/default-conf.yaml - high severity
Identified a HashiCorp Terraform password field, risking unauthorized infrastructure configuration and security breaches.
View details in Aikido Security

func setupDummyUserOptions() *starter.UpsertUserOptions {
return &starter.UpsertUserOptions{
Email: "[email protected]",
Password: "test123456789",
Copy link

@aikido-pr-checks aikido-pr-checks bot Oct 17, 2025

Choose a reason for hiding this comment

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

Exposed secret in starter/setup_test.go - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
View details in Aikido Security

zhangshanwen and others added 12 commits October 17, 2025 11:42
Renamed the test file to better reflect its contents or the module it is testing. No changes to the file contents.
Introduces auth_test.go and uset_test.go to cover password change, profile renaming, role-based visibility for roles and users, and basic user listing. These tests ensure correct behavior for different user roles and actions in the starter module.
This commit fixes the style issues introduced in 4a76d33 according to the output
from Gofumpt.

Details: #1030
Changed the user variable in the TOTP revocation event to a pointer for correct usage. Added new test cases for unlocking users, sending reset password emails, revoking TOTP, and editing users to improve test coverage.
Introduced PresetsDataOperatorWithGRPC example and its test to demonstrate gRPC-based DataOperator usage. Registered the new example in mux. Enhanced user tests in starter/uset_test.go to cover user update scenarios, validation, and role assignments.
This commit fixes the style issues introduced in d902341 according to the output
from Gofumpt.

Details: #1030
Deleted the PresetsDataOperatorWithGRPC function and its registration in mux.go as it is no longer used. Updated PresetsListingFilterNotificationFunc to use DataOperatorWithGRPC directly.
Introduced a mockGRPCDataOperatorWrapper to demonstrate custom DataOperator logic with GRPC-like validation in listing.go. Added PresetsDataOperatorWithGRPC example and corresponding tests for update and delete operations in listing_test.go. Registered the new example in mux.go.
Introduces a new test for the auth login page to verify provider buttons are rendered. Updates test environment setup to include page builder for relevant tests. Adds authentication provider keys and secrets to test config.yaml for Google, Microsoft, and GitHub.
This commit fixes the style issues introduced in c7fe83d according to the output
from Gofumpt.

Details: #1030
Set Debug to true in multiple test cases for improved test output visibility. Refactored imports and test utilities usage in auth_test.go, and added new tests for user login and password reset flows to enhance authentication coverage.
// Prepare one Manager user to ensure listing filter can exclude it for non-admin
mgr, err := starter.UpsertUser(context.Background(), db, &starter.UpsertUserOptions{
Email: "[email protected]",
Password: "1234567890abcd",
Copy link

@aikido-pr-checks aikido-pr-checks bot Oct 20, 2025

Choose a reason for hiding this comment

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

Exposed secret in starter/auth_test.go - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
View details in Aikido Security

zhangshanwen and others added 6 commits October 20, 2025 11:31
Added tests to cover successful password reset, failure when performed by the initial user, and password mismatch scenarios. These tests ensure correct authentication behavior and error handling during password reset flows.
Deleted unnecessary comments from password reset test cases to improve code clarity and reduce clutter in the test file.
Replaces the password mismatch test with a test that checks for failure when the new password is too short. The new test expects a panic when attempting to reset the password with insufficient length.
This commit fixes the style issues introduced in 9984f09 according to the output
from Gofumpt.

Details: #1030
Added github.com/qor5/confx as a direct dependency, updated github.com/bodgit/sevenzip to v1.5.2, and removed indirect dependencies on github.com/golang-jwt/jwt/v4 and github.com/qor5/confx. Also updated github.com/nwaples/rardecode/v2 to v2.2.0 in go.sum.
Bump github.com/qor5/x/v3 to a newer version in go.mod and go.sum. Refactor TestDoResetPassword_FailedByInitialUser and TestDoResetPassword_FailedByLessPassword to check for HTTP redirect responses instead of expecting panics, improving test accuracy for reset password failure scenarios.
@zhangshanwen zhangshanwen merged commit 5194403 into main Oct 21, 2025
8 of 10 checks passed
@zhangshanwen zhangshanwen deleted the field-err branch October 21, 2025 02: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.

4 participants