Refactor/56 addapplicationsenabledmiddleware#67
Conversation
# Conflicts: # cmd/api/api.go # cmd/api/applications.go # cmd/api/reviews.go # cmd/api/scans.go # cmd/api/settings.go # docs/docs.go
…//github.com/hackutd/harp into refactor/56-addapplicationsenabledmiddleware
balebbae
left a comment
balebbae
left a comment
There was a problem hiding this comment.
no more importing strconv 😭
| query := ` | ||
| SELECT value | ||
| FROM settings | ||
| WHERE key = 'applications_enabled' |
There was a problem hiding this comment.
Use const and pass it in like the GetAdminScheduleEditEnabled look at line 29
| defer cancel() | ||
|
|
||
| query := ` | ||
| UPDATE settings |
There was a problem hiding this comment.
Instead of update returning do insert ... on conflict do update. look at the other upsert patterns
| return err | ||
| } | ||
|
|
||
| func (s *SettingsStore) GetApplicationsEnabled(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
dont return bool when called should already show enabled. just return error look at the other set signature patterns
| RETURNING value` | ||
|
|
||
| var value bool | ||
| err := s.db.QueryRowContext(ctx, query, strconv.FormatBool(enabled)).Scan(&value) |
There was a problem hiding this comment.
don't use strconv use json.Marshal like the other setters. So remove the import as well.
| // | ||
| // @Summary Set applications enabled status | ||
| // @Description Sets whether the application portal is currently open for submissions. Requires SuperAdmin privileges. | ||
| // @Tags superadmin |
There was a problem hiding this comment.
tags should be superadmin/settings
| // @Security CookieAuth | ||
| // @Router /superadmin/settings/applications-enabled [put] | ||
| func (app *application) setApplicationsEnabled(w http.ResponseWriter, r *http.Request) { | ||
| enabled, err := strconv.ParseBool(r.URL.Query().Get("enabled")) |
There was a problem hiding this comment.
use JSON body like the other sets. no strconv :) remove that import. Look at line 365-369 for reference
| return | ||
| } | ||
|
|
||
| //NOTE: Following existing design pattern of Get response and Set response structs |
| r.Route("/applications", func(r chi.Router) { | ||
| r.Get("/", app.listApplicationsHandler) | ||
| r.Get("/stats", app.getApplicationStatsHandler) | ||
| r.Get("/enabled", app.getApplicationsEnabled) |
There was a problem hiding this comment.
remove this. this is a duplicate from line 170
…//github.com/hackutd/harp into refactor/56-addapplicationsenabledmiddleware
| return err | ||
| } | ||
|
|
||
| func (s *SettingsStore) GetApplicationsEnabled(ctx context.Context) (bool, error) { |
| @@ -0,0 +1,2 @@ | |||
| INSERT INTO settings (key, value) VALUES ('applications_enabled', 'true'::jsonb) | |||
There was a problem hiding this comment.
migration 16 is for review_assignment_setting on main. Don't edit existing migrations. just make a new one with task migrate-create {name of migration}
There was a problem hiding this comment.
wait I didn't touch any of the existing migrations
…//github.com/hackutd/harp into refactor/56-addapplicationsenabledmiddleware
Merge main into refactor/56-addapplicationsenabledmiddleware, resolving conflicts in mock_store.go and regenerating docs.go.
3 participants
Summary
Refactor: Move ApplicationsEnabled to Settings and wire up middleware
Store Layer
Moved GetApplicationsEnabled / SetApplicationsEnabled from the Application interface → Settings interface in storage.go (these are global toggles, not application CRUD)
Moved implementations from ApplicationsStore (applications.go) → SettingsStore (settings.go)
Moved mock stubs from MockApplicationStore → MockSettingsStore in mock_store.go
API Layer
Wired ApplicationsEnabledMiddleware onto hacker mutation routes (PATCH /me, POST /me/submit, POST /me/resume-upload-url, DELETE /me/resume)
Added unguarded GET /applications/enabled route so the frontend can check if submissions are open
Super admins bypass the middleware check; all other users receive 403 when applications are disabled
Updated getApplicationsEnabled / setApplicationsEnabled handlers to call app.store.Settings.* instead of app.store.Application.*
Tests
Added TestApplicationsEnabledMiddleware covering:
Returns 403 when applications are disabled
Passes through with 200 when applications are enabled
Uses .Once() on mock expectations to prevent cross-test pollution
Misc
Fixed Swagger tags: admin → admin/applications, superadmin → superadmin/applications