feat(actions): add acknowledgement field to decision dialogs#153
Open
isasmendiagus wants to merge 4 commits intomainfrom
Open
feat(actions): add acknowledgement field to decision dialogs#153isasmendiagus wants to merge 4 commits intomainfrom
isasmendiagus wants to merge 4 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdded an optional "acknowledgement" field and an automatic UTC RFC3339 "timestamp" to component filter types; propagated these through backend entities and service logic, frontend modal and store, generated TypeScript models, and recorded changes in the changelog. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant User
end
rect rgba(200,255,200,0.5)
participant Frontend as Frontend UI
participant Store as Component Store
end
rect rgba(255,200,200,0.5)
participant Backend as Backend Service
participant Repo as ScanOSS Repo
end
User->>Frontend: Open FilterActionModal, enter comment + acknowledgement
Frontend->>Store: onConfirm(payload includes acknowledgement)
Store->>Backend: send ComponentFilterDTO (includes acknowledgement, optional fields)
Backend->>Backend: set Timestamp = now().UTC().Format(RFC3339)
Backend->>Repo: AddBomEntry / RemoveBomEntry with ComponentFilter (acknowledgement, timestamp)
Repo-->>Backend: persist result
Backend-->>Store: response
Store-->>Frontend: update UI / close modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/components/FilterActionModal.tsx (1)
241-251: Add explicit label-to-textarea association for accessibility.The new
Acknowledgementlabel and textarea are siblings, so assistive tech may not reliably link them. Consider addingid/htmlFor.♿ Suggested a11y tweak
+ const acknowledgementId = 'filter-acknowledgement'; ... - <Label> + <Label htmlFor={acknowledgementId}> Acknowledgement <span className="text-xs font-normal text-muted-foreground">(optional)</span> </Label> <Textarea + id={acknowledgementId} value={acknowledgement} onChange={(e) => setAcknowledgement(e.target.value)} placeholder="Add an acknowledgement..." />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/FilterActionModal.tsx` around lines 241 - 251, The Label and Textarea for the Acknowledgement field are not explicitly associated which harms accessibility; update the Textarea and Label usage in the FilterActionModal component by giving the Textarea a unique id (e.g., "acknowledgement") and set the Label's htmlFor to that id so assistive tech can link the label to the input; ensure you keep using the existing state variables acknowledgement and setAcknowledgement and preserve the placeholder/onChange behavior.frontend/src/modules/components/stores/useComponentFilterStore.ts (1)
89-117: Consider extracting a shared DTO builder to reduce drift risk.Both branches now replicate the same common fields (
action/comment/acknowledgement/license). A small helper would make future schema updates safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/modules/components/stores/useComponentFilterStore.ts` around lines 89 - 117, Extract a shared DTO builder function (e.g., buildComponentFilterDTO or makeComponentFilterDTO) that accepts the common values (action, comment, acknowledgement, license) and returns a base object typed as entities.ComponentFilterDTO; then use that builder in both branches when constructing dto arrays and merge branch-specific fields (purl, path, replace_with) into the base before calling FilterComponents. Replace the inline object literals in the folder branch and the selectedResults.map branch with calls to the new builder so future schema changes affect a single location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/FilterActionModal.tsx`:
- Around line 241-251: The Label and Textarea for the Acknowledgement field are
not explicitly associated which harms accessibility; update the Textarea and
Label usage in the FilterActionModal component by giving the Textarea a unique
id (e.g., "acknowledgement") and set the Label's htmlFor to that id so assistive
tech can link the label to the input; ensure you keep using the existing state
variables acknowledgement and setAcknowledgement and preserve the
placeholder/onChange behavior.
In `@frontend/src/modules/components/stores/useComponentFilterStore.ts`:
- Around line 89-117: Extract a shared DTO builder function (e.g.,
buildComponentFilterDTO or makeComponentFilterDTO) that accepts the common
values (action, comment, acknowledgement, license) and returns a base object
typed as entities.ComponentFilterDTO; then use that builder in both branches
when constructing dto arrays and merge branch-specific fields (purl, path,
replace_with) into the base before calling FilterComponents. Replace the inline
object literals in the folder branch and the selectedResults.map branch with
calls to the new builder so future schema changes affect a single location.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8b18a40-59db-42ac-840d-d9408a10d991
📒 Files selected for processing (6)
backend/entities/component.gobackend/entities/scanoss_settings.gobackend/service/component_service_impl.gofrontend/src/components/FilterActionModal.tsxfrontend/src/modules/components/stores/useComponentFilterStore.tsfrontend/wailsjs/go/models.ts
isasmendiagus
force-pushed
the
feat/SP-4172/add-acknowledgement-input
branch
from
3091d08 to
b9f0245
Compare
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/service/component_service_impl.go (1)
70-103: Refactor repeated DTO mapping insetInitialFiltersto a shared converter.The include/remove/replace mapping blocks are mostly identical; a helper would reduce duplication and prevent future field drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/service/component_service_impl.go` around lines 70 - 103, Refactor the repeated mapping in setInitialFilters by extracting a helper function (e.g., mapToComponentFilterDTO) that accepts a source filter and an action (entities.Include/Remove/Replace) and returns entities.ComponentFilterDTO; ensure the helper maps Path, Purl, Usage (string()), Comment, Acknowledgement, License and conditionally sets ReplaceWith when action == entities.Replace, then replace the three for-loops over initialFilters.Include/Remove/Replace with calls that pass each item and the appropriate action into this helper and append the returned DTO to s.initialFilters.frontend/src/components/FilterActionModal.tsx (1)
233-245: Consider extracting the repeated “label + tooltip” block into a small helper component.This duplication is small now, but extracting it will reduce drift when styles/text behavior evolve.
Also applies to: 256-268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/FilterActionModal.tsx` around lines 233 - 245, The repeated "label + tooltip" JSX block in FilterActionModal (the fragment using Label, Tooltip, TooltipTrigger, TooltipContent and HelpCircle) should be extracted into a small presentational component (e.g., ActionLabelWithTooltip or LabelTooltip) and reused in both places (current block and the one at lines ~256-268). Create the new component accepting props for label text, optional subtext (the "(optional)" span), tooltip content and any className overrides; replace the duplicated JSX in FilterActionModal with <ActionLabelWithTooltip ... /> to keep markup and tooltip behavior consistent and reduce drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/FilterActionModal.tsx`:
- Around line 238-240: The HelpCircle SVG used as the child of TooltipTrigger is
not focusable or announced to screen readers; update the TooltipTrigger usage in
FilterActionModal so its child is a native focusable control (a <button
type="button">) with an appropriate aria-label (e.g., aria-label="Help for
[context]"). Move the existing className and hover styles to that button (keep
TooltipTrigger asChild) and ensure the button contains the HelpCircle icon;
apply the same change for the second occurrence around the HelpCircle at the
other location referenced (lines ~261-263). This will make TooltipTrigger
keyboard-accessible and screen-reader friendly.
---
Nitpick comments:
In `@backend/service/component_service_impl.go`:
- Around line 70-103: Refactor the repeated mapping in setInitialFilters by
extracting a helper function (e.g., mapToComponentFilterDTO) that accepts a
source filter and an action (entities.Include/Remove/Replace) and returns
entities.ComponentFilterDTO; ensure the helper maps Path, Purl, Usage
(string()), Comment, Acknowledgement, License and conditionally sets ReplaceWith
when action == entities.Replace, then replace the three for-loops over
initialFilters.Include/Remove/Replace with calls that pass each item and the
appropriate action into this helper and append the returned DTO to
s.initialFilters.
In `@frontend/src/components/FilterActionModal.tsx`:
- Around line 233-245: The repeated "label + tooltip" JSX block in
FilterActionModal (the fragment using Label, Tooltip, TooltipTrigger,
TooltipContent and HelpCircle) should be extracted into a small presentational
component (e.g., ActionLabelWithTooltip or LabelTooltip) and reused in both
places (current block and the one at lines ~256-268). Create the new component
accepting props for label text, optional subtext (the "(optional)" span),
tooltip content and any className overrides; replace the duplicated JSX in
FilterActionModal with <ActionLabelWithTooltip ... /> to keep markup and tooltip
behavior consistent and reduce drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e1b8dce-14a7-45f6-b8d6-50352897ed3e
📒 Files selected for processing (7)
CHANGELOG.mdbackend/entities/component.gobackend/entities/scanoss_settings.gobackend/service/component_service_impl.gofrontend/src/components/FilterActionModal.tsxfrontend/src/modules/components/stores/useComponentFilterStore.tsfrontend/wailsjs/go/models.ts
✅ Files skipped from review due to trivial changes (3)
- CHANGELOG.md
- frontend/wailsjs/go/models.ts
- backend/entities/component.go
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/entities/scanoss_settings.go
- frontend/src/modules/components/stores/useComponentFilterStore.ts
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
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
1 participant
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 by CodeRabbit
New Features
Documentation