Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for privacyNoticeText and privacyNoticeUrl to AiInformation by downgrading the @instructure/ui-instructure dependency from version 11.2.0 to 10.29.0, and updating the NutritionFactsExtended component to support additional links.
Changes:
- Downgraded
@instructure/ui-instructurefrom v11.2.0 to v10.29.0 to access new privacy notice properties - Added
privacyNoticeTextandprivacyNoticeUrlfields to the Entry type and all generated components - Applied code style improvements (TypeScript import syntax, formatting, type definitions)
Reviewed changes
Copilot reviewed 64 out of 66 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Downgraded @instructure/ui-instructure from 11.2.0 to 10.29.0 and added dependencies for v10.29.0 |
| packages/aiinfo/package.json | Updated version from 3.0.0-alpha.4 to 2.10.0 and dependency version |
| packages/aiinfo/types.ts | Added privacyNoticeText and privacyNoticeUrl to Entry interface |
| packages/aiinfo/utils/*.ts | Added privacy notice fields to data transformation and formatting utilities |
| packages/aiinfo/node/components/*/index.ts | Added privacy notice fields to all component exports (empty strings for most, actual values for igniteagent) |
| packages/aiinfo/src/*.{mjs,cjs,d.mts,d.cts} | Generated output files with privacy notice fields included |
| Various test files | Updated tests to include privacy notice fields and improved code formatting |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 80 out of 85 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/aiinfo/utils/writeChangelog.test.ts:1
- The new test added at line 109-155 verifies that privacy notice fields are included in entries, but there's no test coverage for the writeChangelog function handling these new fields in changed entries. Consider adding a test case that verifies changelog generation when privacyNoticeText or privacyNoticeUrl values change.
| let Feature: ExtendedNutritionFactsProps | undefined = undefined; | ||
| if (product && isNutritionFacts(product?.nutritionFacts)) { | ||
| Feature = extendedNutritionFacts(product); | ||
| console.log(typeof Feature.description); |
There was a problem hiding this comment.
Debug console.log statement should be removed before merging to production.
| console.log(typeof Feature.description); |
| const mapped = data.map((d) => | ||
| Object.assign(d, { | ||
| highlighted: Boolean( | ||
| d.level.endsWith(permStr) && highlightId === d.level.replace(`LEVEL `, `L`), | ||
| ), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Using Object.assign mutates the original data array elements. This creates a side effect where the source strings.data is modified. Use spread operator instead: data.map((d) => ({ ...d, highlighted: ... })) to avoid mutation.
1 participant
@instructure.ai/aiinfo: Add support forprivacyNoticeTextandprivacyNoticeUrlto AiInformation from INSTUI @ 10.29.0@instructure.ai/nutritionfacts: Update NutritionFactsExtended to support description, and additional links.