slack-bot add optional Activity Type to Slack modals and Jira filing#5115
slack-bot add optional Activity Type to Slack modals and Jira filing#5115deepsm007 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughAdds an "Activity Type" selection to Slack modals, validates allowed values, threads a CLI-configurable Jira custom field ID into the IssueFiler, and includes the activity type when creating Jira issues across bug, consultation, enhancement, and incident flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SlackModal
participant Handler
participant Validator
participant IssueFiler
participant Jira
User->>SlackModal: Fill form (select Activity Type + fields)
SlackModal->>Handler: Submit payload
Handler->>Handler: Extract values (including ActivityType)
Handler->>Validator: IsValidActivityType(value)
Validator-->>Handler: valid / invalid
Handler->>IssueFiler: FileIssue(..., activityType, ...)
IssueFiler->>IssueFiler: Validate & set custom field (if configured)
IssueFiler->>Jira: Create issue request (includes custom field)
Jira-->>IssueFiler: Issue created
IssueFiler-->>Handler: Success
Handler-->>SlackModal: Acknowledge
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/slack/modals/activity_type.go (1)
20-38: Minor: Unnecessary variable assignment in loop.The assignment
v := oon line 31 is redundant sinceois already a string value (not a pointer or reference type). You can useodirectly.♻️ Suggested simplification
func activityTypeSelectOptions() []*slack.OptionBlockObject { opts := []string{ ActivityTypeAssociateWellness, ActivityTypeFutureSustainability, ActivityTypeQualityStabilityReliability, ActivityTypeProductPortfolioWork, ActivityTypeSecurityCompliance, ActivityTypeIncidentsSupport, } out := make([]*slack.OptionBlockObject, 0, len(opts)) for _, o := range opts { - v := o out = append(out, &slack.OptionBlockObject{ - Text: &slack.TextBlockObject{Type: slack.PlainTextType, Text: v}, - Value: v, + Text: &slack.TextBlockObject{Type: slack.PlainTextType, Text: o}, + Value: o, }) } return out }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/slack/modals/activity_type.go` around lines 20 - 38, In activityTypeSelectOptions, remove the redundant loop variable assignment v := o and use o directly when building the slack.OptionBlockObject (replace usages of v with o); this simplifies the loop in activityTypeSelectOptions and avoids an unnecessary local variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/slack/modals/activity_type.go`:
- Around line 20-38: In activityTypeSelectOptions, remove the redundant loop
variable assignment v := o and use o directly when building the
slack.OptionBlockObject (replace usages of v with o); this simplifies the loop
in activityTypeSelectOptions and avoids an unnecessary local variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 853e6733-ca65-42a4-a510-cfbf51fbbfec
📒 Files selected for processing (16)
cmd/slack-bot/main.gopkg/jira/activity_type.gopkg/jira/activity_type_test.gopkg/jira/fake.gopkg/jira/issues.gopkg/slack/modals/activity_type.gopkg/slack/modals/bug/view.gopkg/slack/modals/bug/view_test.gopkg/slack/modals/consultation/view.gopkg/slack/modals/consultation/view_test.gopkg/slack/modals/enhancement/view.gopkg/slack/modals/enhancement/view_test.gopkg/slack/modals/handlers.gopkg/slack/modals/incident/view.gopkg/slack/modals/incident/view_test.gopkg/slack/modals/payload_test.go
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepsm007, Prucek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
/retest-required |
deepsm007
force-pushed
the
slack-modals-activity-type-defaults
branch
from
35e5221 to
6e84cd5
Compare
|
New changes are detected. LGTM label has been removed. |
deepsm007
force-pushed
the
slack-modals-activity-type-defaults
branch
from
6e84cd5 to
75d387c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/slack/modals/activity_type.go (1)
29-36: Minor: Unnecessary variable copy.The local variable
v := oon line 31 is unnecessary sinceois already a string value (not a loop variable capturing a pointer). You can useodirectly in the struct literal.♻️ Suggested simplification
out := make([]*slack.OptionBlockObject, 0, len(opts)) for _, o := range opts { - v := o out = append(out, &slack.OptionBlockObject{ - Text: &slack.TextBlockObject{Type: slack.PlainTextType, Text: v}, - Value: v, + Text: &slack.TextBlockObject{Type: slack.PlainTextType, Text: o}, + Value: o, }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/slack/modals/activity_type.go` around lines 29 - 36, The loop in activity_type.go creates an unnecessary local copy v := o before building each *slack.OptionBlockObject; remove the redundant assignment and use the loop variable o directly when constructing the TextBlockObject and Value (i.e., replace references to v with o in the loop that produces out := make([]*slack.OptionBlockObject, ...) so you only append &slack.OptionBlockObject{ Text: &slack.TextBlockObject{Type: slack.PlainTextType, Text: o}, Value: o }).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/jira/issues.go`:
- Around line 98-110: The Jira custom select field is being set using
jira.Option (in the block guarded by activityType != "" and
f.activityTypeCustomField) but go-jira expects a map with "id" or "value" keys;
update the assignment to toCreate.Fields.Unknowns[f.activityTypeCustomField] =
map[string]interface{}{"value": activityType} (keeping the existing Nil-check
that creates tcontainer.NewMarshalMap()) so the custom field is serialized in
the correct format.
---
Nitpick comments:
In `@pkg/slack/modals/activity_type.go`:
- Around line 29-36: The loop in activity_type.go creates an unnecessary local
copy v := o before building each *slack.OptionBlockObject; remove the redundant
assignment and use the loop variable o directly when constructing the
TextBlockObject and Value (i.e., replace references to v with o in the loop that
produces out := make([]*slack.OptionBlockObject, ...) so you only append
&slack.OptionBlockObject{ Text: &slack.TextBlockObject{Type:
slack.PlainTextType, Text: o}, Value: o }).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a5f13037-02f6-4660-a662-d5d85f11cb81
📒 Files selected for processing (17)
cmd/slack-bot/main.gogo.modpkg/jira/activity_type.gopkg/jira/activity_type_test.gopkg/jira/fake.gopkg/jira/issues.gopkg/slack/modals/activity_type.gopkg/slack/modals/bug/view.gopkg/slack/modals/bug/view_test.gopkg/slack/modals/consultation/view.gopkg/slack/modals/consultation/view_test.gopkg/slack/modals/enhancement/view.gopkg/slack/modals/enhancement/view_test.gopkg/slack/modals/handlers.gopkg/slack/modals/incident/view.gopkg/slack/modals/incident/view_test.gopkg/slack/modals/payload_test.go
✅ Files skipped from review due to trivial changes (3)
- pkg/slack/modals/consultation/view_test.go
- pkg/jira/activity_type_test.go
- pkg/jira/activity_type.go
🚧 Files skipped from review as they are similar to previous changes (7)
- pkg/slack/modals/bug/view_test.go
- pkg/slack/modals/bug/view.go
- pkg/slack/modals/enhancement/view_test.go
- pkg/slack/modals/handlers.go
- pkg/slack/modals/payload_test.go
- pkg/jira/fake.go
- pkg/slack/modals/enhancement/view.go
| if activityType != "" { | ||
| if !IsValidActivityType(activityType) { | ||
| return nil, fmt.Errorf("invalid activity type %q: must be one of the configured Slack options", activityType) | ||
| } | ||
| if f.activityTypeCustomField == "" { | ||
| logger.WithField("activity_type", activityType).Warn("Activity Type is set but Jira custom field id is not configured; omitting from Jira API request") | ||
| } else { | ||
| if toCreate.Fields.Unknowns == nil { | ||
| toCreate.Fields.Unknowns = tcontainer.NewMarshalMap() | ||
| } | ||
| toCreate.Fields.Unknowns[f.activityTypeCustomField] = jira.Option{Value: activityType} | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
go-jira library custom field select option format
💡 Result:
In the go-jira library (github.com/andygrunwald/go-jira), custom fields, including select options, are handled via the IssueFields.Unknowns field, which is a tcontainer.MarshalMap from github.com/trivago/tgo. For a single-select custom field, set the value as a map with the "id" or "value" key matching the option's ID or display value: unknowns := tcontainer.NewMarshalMap unknowns["customfield_XXXXX"] = map[string]string{"id": "12345"} // or unknowns["customfield_XXXXX"] = map[string]string{"value": "Option Display Name"} Then assign to the issue: i := jira.Issue{ Fields: &jira.IssueFields{ // ... other fields ... Unknowns: unknowns, }, } This matches Jira REST API format for select fields: {"id": "optionId"} or {"value": "optionValue"}. For reading values from existing issues, use issue.Fields.Unknowns.Value("customfield_XXXXX") or issue.GetCustomField("customfield_XXXXX"). Note: v2 is in development with better custom field support, but current stable (v1.x) uses this approach. Use tcontainer.NewMarshalMap instead of plain map[string]interface{} for proper JSON marshaling of nested objects.
Citations:
- 1: How do I handle a custom field that is an object instead of a simple string? andygrunwald/go-jira#608
- 2: Documentation for custom jira fields andygrunwald/go-jira#350
- 3: Support of custom fields while creating issue andygrunwald/go-jira#136
- 4: https://developer.atlassian.com/server/jira-servicedesk/info/field-input-formats/
- 5: https://github.com/andygrunwald/Go-jira
- 6: Feature/example createissue customfields andygrunwald/go-jira#351
- 7: Is support for creating issues with custom fields available? andygrunwald/go-jira#35
- 8: Custom fields not being written, and creating issues returns an incorrect *jira.Issue with nil fields andygrunwald/go-jira#380
- 9: Allow to create an Issue with CustomFields when they are required fields andygrunwald/go-jira#178
Fix custom field format for Jira select fields.
The code uses jira.Option{Value: activityType}, but the go-jira library expects custom select fields to be set as a map with "id" or "value" keys. Change to:
toCreate.Fields.Unknowns[f.activityTypeCustomField] = map[string]interface{}{"value": activityType}The current implementation will fail when creating issues because jira.Option is not the correct format for the Jira REST API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/jira/issues.go` around lines 98 - 110, The Jira custom select field is
being set using jira.Option (in the block guarded by activityType != "" and
f.activityTypeCustomField) but go-jira expects a map with "id" or "value" keys;
update the assignment to toCreate.Fields.Unknowns[f.activityTypeCustomField] =
map[string]interface{}{"value": activityType} (keeping the existing Nil-check
that creates tcontainer.NewMarshalMap()) so the custom field is serialized in
the correct format.
2 participants
/cc @openshift/test-platform
Summary by CodeRabbit
New Features
Tests