feat: add bulk messages history table with status counts#898
Conversation
- Add GET /v1/bulk-messages endpoint that returns the last 10 bulk message batches for the authenticated user - Add optimized SQL query using GROUP BY and FILTER to aggregate status counts (scheduled, pending, sent, delivered, failed) from millions of messages - Add Vuetify data table on /bulk-messages page showing history - Add View action that navigates to /search-messages with the bulk message ID pre-filled as a query parameter - Update search-messages page to read ?query= URL param and auto-fill + auto-search on page load Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 1 high |
🟢 Metrics 38 complexity · 12 duplication
Metric Results Complexity 38 Duplication 12
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Greptile SummaryThis PR adds a bulk message history table to the
Confidence Score: 3/5The API and bulk-messages page changes are solid, but the auto-search feature on the search page has a concurrency issue with Cloudflare Turnstile that needs to be resolved before merging. The SQL aggregation query and new endpoint are well-structured and the bulk-messages UI is straightforward. The main concern is in the search page: the new auto-search path calls fetchMessages(true) from mounted(), which internally sets options.page = 1 and triggers the existing deep watcher on options, causing a second concurrent call to fetchMessages(). Both reach getCaptcha() and attempt to render a Cloudflare Turnstile widget in the same #cloudflare-turnstile DOM element simultaneously — a scenario Turnstile does not handle gracefully and that will surface as CAPTCHA errors for users clicking View from the bulk history table. Additionally, the swagger docs were manually edited to use type names that don't exist in code, which will mislead API consumers using the published spec. web/pages/search-messages/index.vue needs the concurrent-fetch guard; api/docs/docs.go, swagger.json, and swagger.yaml should be regenerated to match the actual Go types rather than the manually-edited names. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant BulkPage as /bulk-messages (Vue)
participant SearchPage as /search-messages (Vue)
participant Store as Vuex Store
participant API as GET /v1/bulk-messages
participant Service as MessageService
participant Repo as gormMessageRepository
participant DB as PostgreSQL
Browser->>BulkPage: Navigate to /bulk-messages
BulkPage->>Store: dispatch('loadUser')
BulkPage->>Store: dispatch('fetchBulkMessageOrders')
Store->>API: GET /v1/bulk-messages
API->>Service: GetBulkMessages(ctx, userID)
Service->>Repo: GetBulkMessages(ctx, userID, 10)
Repo->>DB: "SELECT request_id, COUNT(*) FILTER(WHERE status=...) GROUP BY request_id LIMIT 10"
DB-->>Repo: BulkMessage rows
Repo-->>Service: "[]*entities.BulkMessage"
Service-->>API: "[]*entities.BulkMessage"
API-->>Store: "200 { data: [...] }"
Store-->>BulkPage: orders[]
BulkPage->>Browser: Render history table
Browser->>BulkPage: Click View on a row
BulkPage->>SearchPage: "Navigate to /search-messages?query=bulk-uuid"
SearchPage->>Store: dispatch('loadUser') + loadPhones()
SearchPage->>SearchPage: "formQuery = route.query.query"
SearchPage->>SearchPage: fetchMessages(true) + watcher fetchMessages() concurrent
Reviews (1): Last reviewed commit: "refactor: use StartWithLogger in GetBulk..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async mounted() { | ||
| await this.$store.dispatch('loadUser') | ||
| await this.$store.dispatch('loadPhones') | ||
|
|
||
| // Auto-fill search query from URL params | ||
| const queryParam = this.$route.query.query | ||
| if (queryParam && typeof queryParam === 'string') { | ||
| this.formQuery = queryParam | ||
| } | ||
|
|
||
| this.loading = false | ||
|
|
||
| // Auto-search if query param was provided | ||
| if (this.formQuery) { | ||
| this.fetchMessages(true) | ||
| } |
There was a problem hiding this comment.
When the page is opened with ?query=bulk-xxx, Vuetify's v-data-table emits update:options on mount (to sync its internal defaults), which triggers the existing deep watcher and calls fetchMessages(). Immediately after, mounted() also calls fetchMessages(true). Both paths reach getCaptcha() concurrently and call turnstile.render('#cloudflare-turnstile', ...) on the same container element. Cloudflare Turnstile does not support rendering into the same container twice; the second call typically throws or silently fails, leaving the user with a broken/errored CAPTCHA and a search that never completes.
A straightforward guard is to track whether a fetch is already in flight and skip a new request if one is pending, or move the Turnstile render target out of the shared lifecycle so concurrent renders can't collide.
There was a problem hiding this comment.
Fixed. Added an initialLoadComplete flag that prevents the options deep watcher from triggering fetchMessages() until mounted() finishes. This ensures only a single fetchMessages(true) call is made when the page is opened with a ?query= param, avoiding the concurrent Turnstile render.
| // @Success 200 {object} responses.BulkMessagesResponse | ||
| // @Failure 401 {object} responses.Unauthorized | ||
| // @Failure 500 {object} responses.InternalServerError | ||
| // @Router /bulk-messages [get] |
There was a problem hiding this comment.
The @Success annotation names responses.BulkMessagesResponse, which is the real struct defined in bulk_message_responses.go. However, the generated swagger docs (docs.go, swagger.json, swagger.yaml) reference responses.BulkMessageOrdersResponse and entities.BulkMessageOrder — types that do not exist in the codebase. The entity is entities.BulkMessage and the response is responses.BulkMessagesResponse. The docs appear to have been manually edited with different names, causing them to diverge from the actual code and annotation. API clients consuming the published spec will see schema names that do not match the real response.
There was a problem hiding this comment.
Fixed. Regenerated swagger docs with swag init so the generated types now correctly reference responses.BulkMessagesResponse and entities.BulkMessage.
| var orders []*entities.BulkMessage | ||
| err := repository.db.WithContext(ctx).Raw(` | ||
| SELECT | ||
| request_id, | ||
| COUNT(*) as total, | ||
| COUNT(*) FILTER (WHERE status = 'scheduled') as scheduled_count, | ||
| COUNT(*) FILTER (WHERE status = 'pending') as pending_count, | ||
| COUNT(*) FILTER (WHERE status = 'failed') as failed_count, | ||
| COUNT(*) FILTER (WHERE status = 'sent') as sent_count, | ||
| COUNT(*) FILTER (WHERE status = 'delivered') as delivered_count, | ||
| MIN(created_at) as created_at | ||
| FROM messages | ||
| WHERE user_id = ? AND request_id LIKE 'bulk-%' | ||
| GROUP BY request_id | ||
| ORDER BY MIN(created_at) DESC | ||
| LIMIT ? | ||
| `, userID, limit).Scan(&orders).Error | ||
| if err != nil { | ||
| msg := fmt.Sprintf("cannot fetch bulk message orders for user [%s]", userID) | ||
| return nil, repository.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, msg)) | ||
| } | ||
|
|
||
| return orders, nil |
There was a problem hiding this comment.
null instead of []
var orders []*entities.BulkMessage is declared as a nil slice. When GORM's Scan finds no matching rows it leaves the variable nil; Go's encoding/json then marshals a nil slice as null, not []. The swagger schema marks data as a required array, so clients following the spec and expecting an array will fail on a JSON null. The store action defends against this with ?? [], but external API clients (SDKs, scripts) may not. Initializing orders to an empty slice before the query (orders := make([]*entities.BulkMessage, 0)) ensures the JSON is always [] when empty.
There was a problem hiding this comment.
Fixed. Changed from var declaration to make([]*entities.BulkMessage, 0) so empty results serialize as [] instead of null in JSON.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix double-search CAPTCHA conflict by adding initialLoadComplete guard to prevent the options watcher from firing before mounted completes - Fix nil slice serializing as null by using make() for empty slice init - Regenerate swagger docs to match actual type names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace v-data-table with v-simple-table to match the pattern used in the settings page. Added a short description paragraph explaining what the table shows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add + prefix normalization in validateMessages before phonenumbers.Parse - Change SendTime from *time.Time to string (SendTimeRaw) for CSV unmarshaling - Add GetSendTime() method that parses multiple date formats gracefully - Empty SendTime fields no longer cause 'Cannot read contents' errors - Support RFC3339, YYYY-MM-DDTHH:MM:SS, YYYY-MM-DD HH:MM:SS, YYYY-MM-DD formats Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allows searching by bulk message IDs (e.g. bulk-8dcc3d68-57bd-4913-a5ff-52c107ffc0c9) which are 41 characters long. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- TestBulkSMS_CSV: uploads CSV with 1 message, fires SENT event, verifies history - TestBulkSMS_Excel: uploads Excel with 2 messages, fires DELIVERED on one, verifies mixed status counts - Add shared helpers: uploadBulkFile, fetchBulkMessages, searchMessages, findBulkEntry - Add excelize/v2 dependency for Excel file creation in tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…on tests The searchMessages helper was using 'owners' (plural) param and missing 'contact', but the GET /v1/messages endpoint requires 'owner' (singular) and 'contact' fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Message 2 transitions to 'scheduled' after waitForFCMPush, not 'pending'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The old indexes (owner_1_timestamp_-1, user_id_1) have already been dropped in production, so this migration code is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Summary
Adds a bulk messages history table on the /bulk-messages page and a new API endpoint to power it.
Changes
API:
Web:
Performance
The query filters on user_id + request_id LIKE 'bulk-%' and groups by request_id with a LIMIT 10, making it efficient even on tables with millions of rows.