RidgwaySystems/rs_website
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

add a page for tickets from gitea #2

Merged
blakeridgway merged 2 commits from blake/ticket-updates into main 2026-04-13 03:51:26 -05:00
Conversation 1 Commits 2 Files Changed 16 +782 -12
blakeridgway commented 2026-04-11 13:23:30 -05:00
Owner
Copy Link
No description provided.
blakeridgway added 1 commit 2026-04-11 13:23:32 -05:00
add a page for tickets from gitea 0ba2f7af4e
ridgway-bot commented 2026-04-11 13:24:02 -05:00
First-time contributor
Copy Link

AI Code Review

Code Review

Security & Data Handling

  1. Gitea token exposure risk (internal/handler/handler.go:48-49): The token is read from environment variables, which is good, but ensure the token has minimal required scopes (only "issues" write as documented). Consider adding validation that the token is non-empty when GITEA_URL is set.

  2. Error logging in public endpoint (internal/handler/public.go:196): The log.Printf in the status handler could leak internal error details in production. Consider logging at debug level or suppressing detailed error messages in production.

Logic & Edge Cases

  1. Missing error handling (internal/gitea/gitea.go:125): The json.NewDecoder(resp.Body).Decode(&labels) call has //nolint:errcheck but ignores potential JSON parsing errors. If the response is malformed, the function will proceed to create a duplicate label. Handle the error properly.

  2. Hardcoded label color (internal/handler/admin.go:601): The label color "#e11d48" is hardcoded. Consider making this configurable via environment variable or using a default that matches the Gitea instance's theme.

  3. Issue limit (internal/gitea/gitea.go:88-89): The API limits results to 50 issues. If the repository has more than 50 open issues, some may be missed. Document this limitation or implement pagination.

  4. No validation for Gitea URL (internal/handler/handler.go:48-49): The GITEA_URL is used directly without validation (e.g., ensuring it's a valid URL). Consider adding a basic sanity check.

  5. Concurrent label creation race (internal/gitea/gitea.go:112-150): If two requests simultaneously try to create the same missing label, both may attempt creation, potentially causing an error on the second attempt. The Gitea API may handle this gracefully, but it's worth noting.

Performance

  1. Repeated label fetching (internal/handler/admin.go:601): GetOrCreateLabel is called on every POST request, which involves an API call to list all labels. Consider caching the label ID after first fetch (with appropriate invalidation if the label is deleted).

  2. Public status page latency (internal/handler/public.go:193-199): The Gitea API call to fetch planned outages is synchronous and could slow down the status page if Gitea is slow or unreachable. The 10-second timeout helps, but consider adding a shorter timeout or caching for the public endpoint.

Style & Maintainability

  1. Magic number for HTTP status (internal/gitea/gitea.go:143, 166): Using resp.StatusCode >= 300 is fine, but consider defining constants for expected success codes (e.g., http.StatusCreated for label creation).

  2. Inconsistent error messages (internal/gitea/gitea.go): Error messages sometimes include gitea: prefix, sometimes not. Standardize error formatting.

  3. Unused method (internal/gitea/gitea.go:43-51): Issue.LabelID is defined but never used in the codebase. Remove if not needed, or add a comment if it's for future use.

  4. Missing template for empty state (templates/admin/outages.html:56-58): The "No open issues found" message appears when .Error is empty, but also when .Issues is empty. This is fine, but ensure it's the intended behavior.

  5. CSS class naming (static/css/style.css:616): The class .planned-outages is used in the template, but the CSS defines .planned-outages (plural) while the template uses planned-outages (same). Ensure consistency.

Testing & Documentation

  1. No tests added: The new Gitea client and handlers lack unit tests. Consider adding tests for the API client and the outage tagging flow.

  2. Example environment variables: The .env.example includes example values, which is good. Ensure the documentation (e.g., README) is updated to explain the new feature.

Overall

The implementation is clean and follows the existing patterns. The main concerns are the error handling in GetOrCreateLabel and the potential performance impact on the public status page. Address these before merging.

Reviewed by deepseek-coder

## AI Code Review ## Code Review ### Security & Data Handling 1. **Gitea token exposure risk** (`internal/handler/handler.go:48-49`): The token is read from environment variables, which is good, but ensure the token has minimal required scopes (only "issues" write as documented). Consider adding validation that the token is non-empty when GITEA_URL is set. 2. **Error logging in public endpoint** (`internal/handler/public.go:196`): The `log.Printf` in the status handler could leak internal error details in production. Consider logging at debug level or suppressing detailed error messages in production. ### Logic & Edge Cases 3. **Missing error handling** (`internal/gitea/gitea.go:125`): The `json.NewDecoder(resp.Body).Decode(&labels)` call has `//nolint:errcheck` but ignores potential JSON parsing errors. If the response is malformed, the function will proceed to create a duplicate label. Handle the error properly. 4. **Hardcoded label color** (`internal/handler/admin.go:601`): The label color `"#e11d48"` is hardcoded. Consider making this configurable via environment variable or using a default that matches the Gitea instance's theme. 5. **Issue limit** (`internal/gitea/gitea.go:88-89`): The API limits results to 50 issues. If the repository has more than 50 open issues, some may be missed. Document this limitation or implement pagination. 6. **No validation for Gitea URL** (`internal/handler/handler.go:48-49`): The GITEA_URL is used directly without validation (e.g., ensuring it's a valid URL). Consider adding a basic sanity check. 7. **Concurrent label creation race** (`internal/gitea/gitea.go:112-150`): If two requests simultaneously try to create the same missing label, both may attempt creation, potentially causing an error on the second attempt. The Gitea API may handle this gracefully, but it's worth noting. ### Performance 8. **Repeated label fetching** (`internal/handler/admin.go:601`): `GetOrCreateLabel` is called on every POST request, which involves an API call to list all labels. Consider caching the label ID after first fetch (with appropriate invalidation if the label is deleted). 9. **Public status page latency** (`internal/handler/public.go:193-199`): The Gitea API call to fetch planned outages is synchronous and could slow down the status page if Gitea is slow or unreachable. The 10-second timeout helps, but consider adding a shorter timeout or caching for the public endpoint. ### Style & Maintainability 10. **Magic number for HTTP status** (`internal/gitea/gitea.go:143, 166`): Using `resp.StatusCode >= 300` is fine, but consider defining constants for expected success codes (e.g., `http.StatusCreated` for label creation). 11. **Inconsistent error messages** (`internal/gitea/gitea.go`): Error messages sometimes include `gitea:` prefix, sometimes not. Standardize error formatting. 12. **Unused method** (`internal/gitea/gitea.go:43-51`): `Issue.LabelID` is defined but never used in the codebase. Remove if not needed, or add a comment if it's for future use. 13. **Missing template for empty state** (`templates/admin/outages.html:56-58`): The "No open issues found" message appears when `.Error` is empty, but also when `.Issues` is empty. This is fine, but ensure it's the intended behavior. 14. **CSS class naming** (`static/css/style.css:616`): The class `.planned-outages` is used in the template, but the CSS defines `.planned-outages` (plural) while the template uses `planned-outages` (same). Ensure consistency. ### Testing & Documentation 15. **No tests added**: The new Gitea client and handlers lack unit tests. Consider adding tests for the API client and the outage tagging flow. 16. **Example environment variables**: The `.env.example` includes example values, which is good. Ensure the documentation (e.g., README) is updated to explain the new feature. ### Overall The implementation is clean and follows the existing patterns. The main concerns are the error handling in `GetOrCreateLabel` and the potential performance impact on the public status page. Address these before merging. --- *Reviewed by deepseek-coder*
blakeridgway added 1 commit 2026-04-13 03:49:33 -05:00
add in twitch integration 668530ce37
blakeridgway merged commit b1feff3bbf into main 2026-04-13 03:51:26 -05:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
blakeridgway
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: RidgwaySystems/rs_website#2
No description provided.
Delete Branch "blake/ticket-updates"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?