Skip to content

Commit dc3fa7c

Browse files
committed
refactor: trim verbose comments across branch additions
Shrinks multi-paragraph godocs and orienting prose added on this branch to one-sentence whys. Keeps the non-obvious rationale (TOCTOU, SQLite read-across-write deadlock, echojwt-401-before-license-gate, xorm:"-" hydration, LIKE semantics in search, stale-JWT re-read, paid-bypass gating) and drops narration of what the next line does. No behavioural change.
1 parent 7392aca commit dc3fa7c

15 files changed

Lines changed: 32 additions & 79 deletions

File tree

frontend/src/components/input/FormField.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ describe('FormField', () => {
233233
},
234234
})
235235
const label = wrapper.find('label.two-col')
236-
// for would point to a different id than the slotted control generates,
237-
// so omit it entirely and rely on the label wrapping the control.
236+
// for="" would mismatch the slotted control's id; rely on the label wrapping instead.
238237
expect(label.attributes('for')).toBeUndefined()
239238
expect(label.find('input').exists()).toBe(true)
240239
})

frontend/src/components/input/FormSelect.test.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,7 @@ describe('FormSelect', () => {
155155
},
156156
})
157157
const select = wrapper.find('select')
158-
// Without an explicit value binding, the native select defaults to the
159-
// first option. If the component forced :value="undefined" that default
160-
// would be broken.
158+
// Forcing :value="undefined" would break the native default-to-first-option behavior.
161159
expect((select.element as HTMLSelectElement).value).toBe('')
162160
})
163161

frontend/src/router/index.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -503,16 +503,12 @@ router.beforeEach(async (to, from) => {
503503
await authStore.checkAuth()
504504

505505
if (to.meta?.requiresAdminPanel) {
506-
// On a hard refresh or direct navigation to /admin, this guard runs
507-
// before loadApp() has fetched /info, so enabledProFeatures is still
508-
// the default empty array. Await the in-flight startup so the license
509-
// check sees the real state instead of silently redirecting.
506+
// Await loadApp() so the license check doesn't race the empty default on direct /admin navigation.
510507
const baseStore = useBaseStore()
511508
await baseStore.appReady
512509
const configStore = useConfigStore()
513510
const featureOn = configStore.isProFeatureEnabled('admin_panel')
514-
// `isAdmin` comes from `/user`, not the JWT — fetch directly in case
515-
// the concurrent checkAuth() was debounced and the store is still hydrating.
511+
// isAdmin comes from /user, not the JWT; force-fetch in case checkAuth() was debounced.
516512
if (authStore.info?.isAdmin === undefined) {
517513
await authStore.refreshUserInfo()
518514
}

frontend/src/stores/base.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,7 @@ export const useBaseStore = defineStore('base', () => {
152152
// and only use a previously stored API URL from localStorage.
153153
const storedApiUrl = localStorage.getItem('API_URL')
154154
if (storedApiUrl) {
155-
// Run the stored URL through checkAndSetApiUrl so /info is
156-
// fetched before we mark the app ready. Without this,
157-
// configStore.enabledProFeatures stays empty for returning
158-
// desktop users and every pro-feature gate (admin panel,
159-
// etc.) fails for the whole session.
155+
// Hydrate /info before marking ready; otherwise pro-feature gates stay off for returning desktop users.
160156
await checkAndSetApiUrl(storedApiUrl)
161157
await authStore.checkAuth()
162158
}
@@ -182,8 +178,7 @@ export const useBaseStore = defineStore('base', () => {
182178
}
183179
}
184180

185-
// Track the in-flight loadApp() so router guards can await config/auth
186-
// hydration instead of racing the default empty state on direct navigation.
181+
// Exposed so router guards can await config/auth hydration on direct navigation.
187182
const appReady = loadApp()
188183

189184
return {

frontend/src/views/admin/UsersView.vue

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -377,9 +377,7 @@ function goToPage(page: number) {
377377
}
378378
379379
const onSearch = useDebounceFn(() => {
380-
// A new search must restart at page 1 — the previous page index is
381-
// meaningless against a different result set and would stick the UI on an
382-
// empty page when the new search is narrower.
380+
// Reset to page 1 so a narrower search doesn't strand the UI on an empty page.
383381
currentPage.value = 1
384382
load()
385383
}, 300)

pkg/cmd/user.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,7 @@ var userSetAdminCmd = &cobra.Command{
123123
initialize.FullInit()
124124
},
125125
Run: func(_ *cobra.Command, args []string) {
126-
// The is_admin flag only unlocks behavior that lives behind the admin
127-
// panel entitlement; setting it on a free instance would hand out paid
128-
// capabilities without the license check used everywhere else.
126+
// Refuse on a free instance; the is_admin bypass is gated by the admin-panel entitlement everywhere else.
129127
if !license.IsFeatureEnabled(license.FeatureAdminPanel) {
130128
log.Fatalf("The admin-panel license feature is not active; refusing to change the is_admin flag.")
131129
}

pkg/routes/api/v1/admin/user_create.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ import (
2929
"github.com/labstack/echo/v5"
3030
)
3131

32-
// CreateUserBody is the payload accepted by the admin create-user endpoint.
33-
// It wraps user.APIUserPassword (username/password/email with the usual
34-
// validation) and adds the admin-only fields that only make sense here.
32+
// CreateUserBody wraps user.APIUserPassword with admin-only fields.
3533
type CreateUserBody struct {
3634
// The full name of the new user. Optional.
3735
Name string `json:"name"`
@@ -91,9 +89,7 @@ func CreateUser(c *echo.Context) error {
9189
newUser.IsAdmin = true
9290
}
9391

94-
// CreateUser flips status to EmailConfirmationRequired only when the mailer
95-
// is on; otherwise the user is already Active and this is a no-op. Force
96-
// Active when the admin asked to skip, or when no mailer is configured.
92+
// Force Active when the admin asked to skip, or when no mailer exists to send the confirmation.
9793
if body.SkipEmailConfirm || !config.MailerEnabled.GetBool() {
9894
if err := user.SetUserStatus(s, newUser, user.StatusActive); err != nil {
9995
_ = s.Rollback()

pkg/routes/api/v1/admin/users.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"xorm.io/xorm"
2525
)
2626

27-
// User re-exposes fields hidden by the default User JSON view.
27+
// User re-exposes fields hidden by the default user.User JSON view.
2828
type User struct {
2929
*user.User
3030
IsAdmin bool `json:"is_admin"`
@@ -64,9 +64,7 @@ func resolveAuthProvider(u *user.User, providers []*openid.Provider) string {
6464
return u.Issuer
6565
}
6666

67-
// UserList is the CRUDable wrapper backing the admin list-users route via
68-
// handler.ReadAllWeb. Only ReadAll is used; everything else is gated by
69-
// the RequireInstanceAdmin middleware.
67+
// UserList backs the admin list-users route via handler.ReadAllWeb; only ReadAll is used.
7068
type UserList struct{}
7169

7270
// ReadAll returns paginated users, optionally filtered by username/email.

pkg/routes/api/v1/admin/users_admin.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ import (
2828
)
2929

3030
type IsAdminPatch struct {
31-
// Pointer so a missing field binds as nil rather than the zero value.
32-
// Without this an empty body would silently demote the target user.
31+
// Pointer to distinguish "omitted" from false; an empty body would silently demote otherwise.
3332
IsAdmin *bool `json:"is_admin"`
3433
}
3534

pkg/routes/api/v1/admin/users_mgmt.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ import (
2828
)
2929

3030
type StatusPatch struct {
31-
// Pointer so a missing field binds as nil rather than StatusActive.
32-
// Without this an empty body would silently re-enable the target user.
31+
// Pointer to distinguish "omitted" from StatusActive; an empty body would silently re-enable otherwise.
3332
Status *user.Status `json:"status"`
3433
}
3534

@@ -75,8 +74,7 @@ func PatchStatus(c *echo.Context) error {
7574
return user.ErrUserDoesNotExist{UserID: id}
7675
}
7776

78-
// Any non-Active status blocks login, so moving an admin out of Active is
79-
// equivalent to demoting them — email-confirmation included.
77+
// Any non-Active status blocks login, so moving an admin out of Active is equivalent to demotion.
8078
if target.IsAdmin && newStatus != user.StatusActive {
8179
if err := user.GuardLastAdmin(s, target); err != nil {
8280
_ = s.Rollback()
@@ -92,7 +90,7 @@ func PatchStatus(c *echo.Context) error {
9290
return err
9391
}
9492

95-
// Refresh locally; GetUserByID refuses disabled accounts.
93+
// Refresh locally since GetUserByID refuses disabled accounts.
9694
target.Status = newStatus
9795
providers, err := openid.GetAllProviders()
9896
if err != nil {

0 commit comments

Comments
 (0)