[LFXV2-1299] feat: add update_note to update meeting endpoint#143
Open
andrest50 wants to merge 5 commits into
Open
[LFXV2-1299] feat: add update_note to update meeting endpoint#143andrest50 wants to merge 5 commits into
andrest50 wants to merge 5 commits into
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds an optional UpdateNote field through the API DSL, domain model, service mapping, and ITX request model, and ensures the HTTP handler populates it from the incoming payload using a string utility. ChangesUpdateNote propagation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Add optional update_note field (max 500 chars) to the update meeting Goa payload and pass it through to the ITX service as the note field on the meeting update request, enabling organizer notes to be included in registrant notification emails. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <[email protected]>
andrest50
force-pushed
the
feat/LFXV2-1299-add-update-note-to-update-meeting
branch
from
a9b326b to
1db3ad0
Compare
There was a problem hiding this comment.
Pull request overview
Adds an optional update_note field to the ITX meeting update API payload and plumbs it through to the ITX request so it can be included in registrant notification emails. The PR also contains operational/indexing-related changes (new reindex script, additional tags/parent refs, and event handler project enrichment) that broaden scope beyond the described endpoint change.
Changes:
- Add optional
update_note(max 500 chars) toPUT /itx/meetings/{meeting_id}and pass through to ITX asnote. - Enrich past-meeting-related events with additional project tags/parent references and populate project fields in summary/attachment handlers.
- Add
scripts/reindex_meetingsoperational tool and extend the local pre-commit hook (gofmt + license header checks).
Reviewed changes
Copilot reviewed 11 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/reindex_meetings/main.go | New operational tool to scroll OpenSearch by object_type and re-put matching NATS KV entries. |
| scripts/reindex_meetings/README.md | Documentation for running the reindex script (dry-run vs. -reindex). |
| scripts/hooks/pre-commit | Adds gofmt verification and a staged-file license header check. |
| pkg/models/itx/meetings.go | Extends ITX request model with note field. |
| internal/service/itx/meeting_service.go | Maps domain UpdateNote into ITX CreateZoomMeetingRequest.Note. |
| internal/domain/models/itx_meeting.go | Adds UpdateNote to the internal meeting request model. |
| internal/domain/models/event_models.go | Adds project slug tags and project parent refs for multiple past-meeting artifact types. |
| gen/meeting_service/service.go | Regenerates Goa service types to include UpdateNote in update payload. |
| gen/http/openapi.yaml | Regenerates OpenAPI schema, adding update_note (maxLength 500) and updating examples. |
| gen/http/meeting_service/server/types.go | Adds UpdateNote field + validation to the update request body. |
| gen/http/meeting_service/client/types.go | Adds UpdateNote support to the generated client request types. |
| gen/http/meeting_service/client/cli.go | Updates generated CLI builders/examples; adds update_note validation in CLI payload build. |
| gen/http/cli/meeting_service/cli.go | Updates generated CLI usage text/examples to reflect regenerated schema. |
| design/meeting-svc.go | Adds UpdateNoteAttribute() to the update meeting payload design. |
| design/itx_types.go | Defines UpdateNoteAttribute() with max length constraint. |
| cmd/meeting-api/eventing/summary_event_handler.go | Enriches summary events with project info from parent past meeting KV; adjusts retry/skip behavior. |
| cmd/meeting-api/eventing/participant_event_handler.go | Removes duplicated KV helper in favor of shared helper. |
| cmd/meeting-api/eventing/kv_helpers.go | New shared helper to fetch parent past meeting project fields from v1 KV. |
| cmd/meeting-api/eventing/attachment_event_handler.go | Enriches attachment events with project mapping/slug and skips when unmapped. |
| cmd/meeting-api/api_itx_meetings.go | Wires API UpdateNote into domain request for meeting update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/domain/models/event_models.go (1)
1216-1217: LGTM - ProjectUID and ProjectSlug fields added to PastMeetingAttachmentEventData.Note:
ProjectSlugis missingomitemptyin the JSON tag unlikeProjectUID. This inconsistency means emptyProjectSlugwill serialize as"project_slug":""while emptyProjectUIDwill be omitted.🔧 Consider adding omitempty for consistency
- ProjectUID string `json:"project_uid"` - ProjectSlug string `json:"project_slug"` + ProjectUID string `json:"project_uid,omitempty"` + ProjectSlug string `json:"project_slug,omitempty"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/domain/models/event_models.go` around lines 1216 - 1217, PastMeetingAttachmentEventData has inconsistent JSON tags: ProjectUID uses `json:"project_uid,omitempty"` but ProjectSlug is `json:"project_slug"` causing empty ProjectSlug to serialize while ProjectUID is omitted; update the ProjectSlug struct tag to `json:"project_slug,omitempty"` to match ProjectUID (locate the fields ProjectUID and ProjectSlug inside the PastMeetingAttachmentEventData struct) and run/adjust any serialization tests that assume the previous behavior.scripts/reindex_meetings/main.go (1)
152-152: Unused parameternatsURLin function signature.The second parameter
natsURLis named_indicating it's unused. If it's not needed, consider removing it from the function signature for clarity.🔧 Suggested fix
-func run(ctx context.Context, nc *nats.Conn, _, osURL string, requestedTypes []string, reindex bool) int { +func run(ctx context.Context, nc *nats.Conn, osURL string, requestedTypes []string, reindex bool) int {And update the call site at line 147:
- exitCode := run(ctx, nc, natsURL, osURL, requestedTypes, *reindex) + exitCode := run(ctx, nc, osURL, requestedTypes, *reindex)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/reindex_meetings/main.go` at line 152, The function signature for run currently includes an unused second parameter named `_` (intended for natsURL); remove that unused parameter from the run function signature and update every call site that invokes run (where natsURL was previously passed) to stop passing that argument so the parameter list matches; locate the run function and its invocations by the symbol run(ctx context.Context, nc *nats.Conn, ...) and adjust both definition and callers accordingly to eliminate the unused natsURL parameter.scripts/hooks/pre-commit (1)
29-41: Minor: Leading newline inmissing_licenseoutput.The string is built with
\n $fwhich prepends a newline before each file. On Line 45,printf "%b\n"will output a leading blank line before the first file path. This is cosmetic but could be cleaner.🔧 Suggested fix to avoid leading newline
missing_license="" for f in $staged_checkable_files; do # Skip excluded paths case "$f" in gen/*|cmd/meeting-api/kodata/*) continue ;; esac if ! grep -qF "$COPYRIGHT" "$f"; then - missing_license="$missing_license\n $f" + if [ -z "$missing_license" ]; then + missing_license=" $f" + else + missing_license="$missing_license\n $f" + fi fi done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/pre-commit` around lines 29 - 41, The current loop appends "\n $f" to missing_license which produces a leading blank line when printed; change the append logic in the loop that iterates over staged_checkable_files so the first file is added without the leading newline (e.g. check if missing_license is empty and set missing_license=" $f" for the first match, otherwise append "\n $f"), leaving the grep -qF "$COPYRIGHT" check and the printf "%b\n" usage intact; locate the variable missing_license and the for f in $staged_checkable_files loop to apply this conditional append fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/domain/models/event_models.go`:
- Around line 1216-1217: PastMeetingAttachmentEventData has inconsistent JSON
tags: ProjectUID uses `json:"project_uid,omitempty"` but ProjectSlug is
`json:"project_slug"` causing empty ProjectSlug to serialize while ProjectUID is
omitted; update the ProjectSlug struct tag to `json:"project_slug,omitempty"` to
match ProjectUID (locate the fields ProjectUID and ProjectSlug inside the
PastMeetingAttachmentEventData struct) and run/adjust any serialization tests
that assume the previous behavior.
In `@scripts/hooks/pre-commit`:
- Around line 29-41: The current loop appends "\n $f" to missing_license which
produces a leading blank line when printed; change the append logic in the loop
that iterates over staged_checkable_files so the first file is added without the
leading newline (e.g. check if missing_license is empty and set
missing_license=" $f" for the first match, otherwise append "\n $f"), leaving
the grep -qF "$COPYRIGHT" check and the printf "%b\n" usage intact; locate the
variable missing_license and the for f in $staged_checkable_files loop to apply
this conditional append fix.
In `@scripts/reindex_meetings/main.go`:
- Line 152: The function signature for run currently includes an unused second
parameter named `_` (intended for natsURL); remove that unused parameter from
the run function signature and update every call site that invokes run (where
natsURL was previously passed) to stop passing that argument so the parameter
list matches; locate the run function and its invocations by the symbol run(ctx
context.Context, nc *nats.Conn, ...) and adjust both definition and callers
accordingly to eliminate the unused natsURL parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1045da68-14e7-44d3-b9a8-e192fd2c0186
⛔ Files ignored due to path filters (9)
gen/http/cli/meeting_service/cli.gois excluded by!**/gen/**gen/http/meeting_service/client/cli.gois excluded by!**/gen/**gen/http/meeting_service/client/types.gois excluded by!**/gen/**gen/http/meeting_service/server/types.gois excluded by!**/gen/**gen/http/openapi.jsonis excluded by!**/gen/**gen/http/openapi.yamlis excluded by!**/gen/**gen/http/openapi3.jsonis excluded by!**/gen/**gen/http/openapi3.yamlis excluded by!**/gen/**gen/meeting_service/service.gois excluded by!**/gen/**
📒 Files selected for processing (14)
cmd/meeting-api/api_itx_meetings.gocmd/meeting-api/eventing/attachment_event_handler.gocmd/meeting-api/eventing/kv_helpers.gocmd/meeting-api/eventing/participant_event_handler.gocmd/meeting-api/eventing/summary_event_handler.godesign/itx_types.godesign/meeting-svc.gointernal/domain/models/event_models.gointernal/domain/models/itx_meeting.gointernal/service/itx/meeting_service.gopkg/models/itx/meetings.goscripts/hooks/pre-commitscripts/reindex_meetings/README.mdscripts/reindex_meetings/main.go
💤 Files with no reviewable changes (1)
- cmd/meeting-api/eventing/participant_event_handler.go
prabodhcs
previously approved these changes
Apr 17, 2026
Contributor
Author
|
There is an issue with the dependabot config which I'll fix in a separate PR. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
update_notefield (max 500 chars) to thePUT /itx/meetings/{meeting_id}endpoint payloadnoteon theCreateZoomMeetingRequest, which includes it in registrant notification emailsTicket
LFXV2-1299
🤖 Generated with Claude Code