Fix alert delivery timeouts, search filter bugs, and forum anonymous authoring - #1185
Merged
RUKAYAT-CODER merged 2 commits intoJul 30, 2026
Merged
Conversation
…authoring Addresses four issues: rinafcode#994 - AlertingService and SlackService fired outbound HTTP calls with no timeout and no bounded retry, and SlackService.sendAlert swallowed delivery failures in an empty catch with logging commented out. Adds ALERT_DELIVERY_TIMEOUT_MS to every outbound call, bounded jittered-backoff retry on transient 5xx, an alert_delivery_failures_total counter labelled by channel, and replaces the empty catch with a proper Logger.error call. rinafcode#995 - SearchService built the category filter with `course.category IN (:cats)` instead of the spread form `IN (:...cats)`, so TypeORM bound the array as a single scalar parameter and category filtering silently broke. Fixes the spread form and implements the level/language filters declared in SearchFilters but never wired up (added as new nullable/indexed columns on Course via migration, since they didn't exist). rinafcode#997 - SearchService.search()/getAutoComplete() caught any error - including infrastructure failures - and returned an empty result set with a 200, making outages indistinguishable from a genuine zero-result search. Now rethrows: infrastructure errors become a 503 (ServiceUnavailableException), malformed filter values (Postgres 22xxx data-exception SQLSTATE) become a 400, and both increment search_query_failures_total labelled by failure class. rinafcode#990 - ForumController computed `authorId = req.user?.id || 'anonymous'` with no guard, so every write was attributed to a shared 'anonymous' identity and ForumService.vote's collision on that id made vote tallies meaningless. Adds JwtAuthGuard to write handlers, removes the fallback, adds a migration purging/flagging pre-existing anonymous rows and adding a foreign key from forum_votes.authorId to users.id. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Kindly resolve conflict. |
|
@sublime247 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Thank you for contributing to the project. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #994,
Closes #995,
Closes #997,
Closes #990.
AlertingServiceandSlackServicefired outbound Slack/PagerDuty HTTP calls with no timeout, andSlackService.sendAlertswallowed delivery failures in an emptycatchwith logging commented out. AddsALERT_DELIVERY_TIMEOUT_MSto every outbound call, bounded jittered-backoff retry on transient 5xx responses, analert_delivery_failures_totalcounter labelled by channel, and replaces the empty catch withLogger.errorincluding the response status.SearchServicebuilt the category filter withcourse.category IN (:cats)instead of the spread formIN (:...cats), so TypeORM bound the array as a single scalar parameter and category filtering silently broke (masked by the surrounding catch). Fixes the spread form and implements thelevel/languagefilters declared inSearchFiltersbut never wired up — they weren't backed by any column, so a migration adds them tocourse(nullable, indexed, mirroringcategory).SearchService.search()/getAutoComplete()caught any error — including infrastructure failures — and returned an empty result set with a 200, making outages indistinguishable from a genuine zero-result search. Now rethrows: infrastructure errors become a 503 (ServiceUnavailableException), malformed filter values (Postgres22xxxdata-exception SQLSTATE) become a 400, and both incrementsearch_query_failures_totallabelled by failure class. The existing ES→DB fallback path (and its counter) is untouched.ForumControllercomputedauthorId = req.user?.id || 'anonymous'with no guard on any write handler, so every write was attributed to a shared'anonymous'identity, andForumService.vote's lookup by(entityType, entityId, authorId)meant every anonymous voter collided on one row — vote tallies were meaningless. AddsJwtAuthGuardto the write handlers (reads stay public), removes the fallback, and adds a migration that purges/flags pre-existing anonymous rows, recomputes affected vote tallies, and adds a foreign key fromforum_votes.authorIdtousers.id.Test plan
npx tsc --noEmitclean on all touched filesnpx eslintclean on all touched filessrc/monitoring/alerting/alerting.service.spec.ts— timeout, retry/backoff, failure countersrc/slack.service.spec.ts— timeout, retry, no-more-empty-catch, failure countersrc/search/search.service.spec.ts— spread-form SQL/param assertions (single + multi value), 503/400 classification, failure countersrc/forum/forum.controller.spec.ts— real HTTP + realJwtAuthGuard(not mocked) proving anonymous writes get 401 and authenticated writes use the real user id, never'anonymous'src/search/search.service.category-filter.integration.spec.ts— exercises the real TypeORM query builder against Postgres (not mocked), seeding courses across two categories; verified it fails against the original unfixedIN (:cats)form and passes against the fix. Excluded from the defaultnpm testrun like other*.integration.spec.tsfiles in this repo — run explicitly against a live database.