Ba 2298 web package placeholder #222
Conversation
mathieubouhelier
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
WalkthroughThe changes introduce a new field Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. 🔧 ESLint
packages/components/modules/activity-log/web/ActivityLogComponent/DateFilterComponent/types.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/modules/activity-log/web/ActivityLogComponent/DateFilterChip/types.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
mathieubouhelier
force-pushed
the
BA-2298-web-package-placeholder
branch
3 times, most recently
from
e35e1c1 to
3058142
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts (1)
10-27: Add JSDoc comments to improve code documentation.The interfaces are well-structured but would benefit from documentation explaining their purpose and usage.
Here's a suggested improvement:
+/** + * Represents the changes made in a log event + */ export interface LogDiff { image?: string[] banner_image?: string[] biography?: string[] [key: string]: any } +/** + * Represents a single log event with its label and associated changes + */ export interface LogEvent { node: { label: string diff?: LogDiff } } +/** + * Represents a log node containing the action type and related events + */ export interface LogNode { verb: string events: { edges: LogEvent[] } }packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx (1)
37-39: Consider enhancing visual presentation for profile updates.While the text display meets the basic requirement, consider adding visual indicators (icons, different colors) to distinguish profile updates from other activities, making them more noticeable in the activity log.
Example enhancement:
<Typography ml="30px" lineHeight="22px" color={theme.palette.text.secondary} + sx={{ + display: 'flex', + alignItems: 'center', + '& svg': { marginRight: 1 } + }} variant="body2" > + {diff?.image || diff?.banner_image || diff?.biography ? ( + <PersonIcon fontSize="small" color="primary" /> + ) : null} {displayText} </Typography>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx(2 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts(1 hunks)packages/components/schema.graphql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and Lint Packages
- GitHub Check: Component Test Packages
🔇 Additional comments (3)
packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.ts (1)
23-30: LGTM! The events field is well-structured.The addition of the
eventsfield withedgesarray containinglabelanddiffproperties aligns well with the PR objectives to enhance activity logging for user profile updates. The structure follows GraphQL best practices and properly integrates with the connection pattern.packages/components/schema.graphql (1)
15-15: LGTM! The diff field is appropriately typed.The addition of the
difffield usingGenericScalartype is a good choice as it provides flexibility to store various data types needed for different profile update activities.packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx (1)
19-27: LGTM! Well-structured changes for handling profile updates.The changes effectively support the new activity logging requirements:
- Early return for robustness
- Clean abstraction for message generation
- Safe access to nested diff data
packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts
Outdated
Show resolved
Hide resolved
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx
Show resolved
Hide resolved
packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts
Outdated
Show resolved
Hide resolved
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts
Outdated
Show resolved
Hide resolved
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts
Show resolved
Hide resolved
anicioalexandre
added
the
needs changes
mathieubouhelier
force-pushed
the
BA-2298-web-package-placeholder
branch
from
3058142 to
fc62d1b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx (1)
26-27: Extracting first diff edge.Fetching the first edge’s diff is suitable for single-event logs. Consider handling multiple edges if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx(2 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts(1 hunks)packages/components/schema.graphql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.ts
- packages/components/schema.graphql
- packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx (1)
packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts (1) (1)
getUpdateMessage(1:22)
🔇 Additional comments (7)
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx (4)
5-6: Imports look good.Bringing in
getUpdateMessageandLogDiffis consistent with the new activity log flow.
19-19: Early return for missing verb.This guard prevents rendering null logs. Good approach.
21-23: Dynamic log message generation.The
getLogMessagefunction is a concise way to handle diffs with fallback toverbMapping.
38-38: Display text final usage.Correctly renders the computed message in the UI.
packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts (3)
3-4: GraphQL import usage.Switching to GraphQL-generated types is best practice for consistency.
5-9: New type definitions (ActivityLogs, ActivityLogEdges, ActivityLogNode, LogEvent).Good alignment with the updated query fragment, ensuring strong type safety.
10-12: Further type definitions (LogEventEdges, LogEventEdgesNode, LogDiff).Clear typed approach for handling diff properties.
mathieubouhelier
force-pushed
the
BA-2298-web-package-placeholder
branch
from
203a7b4 to
e51e08f
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/components/CHANGELOG.md (1)
3-7: Clarify Activity Log Entry DescriptionThe new version entry for 1.0.24 currently states “Add user activities to the Activity Log.” To improve clarity and traceability, please consider explicitly mentioning that the update logs profile picture changes, profile banner updates, and user bio modifications in the activity log. This additional detail will help future developers understand the scope of the changes at a glance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/DateFilterChip/types.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/DateFilterComponent/types.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx(2 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/index.tsx(1 hunks)packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts(1 hunks)packages/components/package.json(1 hunks)packages/components/schema.graphql(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/package.json
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/components/modules/activity-log/web/ActivityLogComponent/DateFilterChip/types.ts
- packages/components/modules/activity-log/web/ActivityLogComponent/DateFilterComponent/types.ts
- packages/components/modules/activity-log/web/ActivityLogComponent/index.tsx
- packages/components/schema.graphql
- packages/components/modules/activity-log/web/ActivityLogComponent/utils.ts
- packages/components/modules/activity-log/common/graphql/queries/ActivityLogsFragment.ts
- packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/index.tsx
- packages/components/modules/activity-log/web/ActivityLogComponent/LogItem/types.ts
🔇 Additional comments (1)
packages/components/CHANGELOG.md (1)
8-9: Verify Dependency Update InformationThe changelog notes an updated dependency for
@baseapp-frontend/components. Please verify that the version bump aligns with the patch's impact and that all corresponding updates or tests have been completed. This will ensure consistency and prevent potential integration issues across the project.
This PR update the placeholder with activity log changes.
Description
As a Developer, on the Baseapp Activity Log,I would like to add user profile update activities (profile picture, banner, and bio changes) to the Activity Log and create comprehensive documentation or a video tutorial on how to add new activities to the log, In order to ensure the Activity Log is more comprehensive and future developers can easily extend its functionality.
Acceptance Criteria
Profile Update Activities
Add the following user activities to the Activity Log:
Changes to the profile picture
Changes to the profile banner
Updates to the user bio
Display Activities using text per Figma
Display the new activities with the correct timestamp and in chronological order.
Given a user changes their profile picture, when the activity is logged, then it should display in the Activity Log with the appropriate format and timestamp.
Given a user updates their profile banner or bio, when the activity is logged, then it should appear in the Activity Log in chronological order with other activities.
Add the new activities to the current package template (Fork Repository)
Summary by CodeRabbit
New Features
eventsfield for improved insights.difffield, enhancing data representation capabilities for activity logs.Refactor
Chores