Solves problems in "feat: Add Communication History PDF Report feature (#1978)" PR#2080
Solves problems in "feat: Add Communication History PDF Report feature (#1978)" PR#2080magush27 wants to merge 15 commits intocboard-org:masterfrom
Conversation
- Implement automatic tracking of symbol selections and phrases - Add PDF generation service with professional formatting - Create export dialog with date range and filtering options - Add Settings page for Communication History with statistics - Integrate with Redux store for persistent data storage - Include comprehensive test coverage - Add documentation and README This feature enables therapists and educators to: - Track AAC usage patterns over time - Generate professional PDF reports for documentation - Monitor communication progress with statistics - Export filtered reports by date range and user Closes cboard-org#1978
- Remove unused 'metadata' parameter in PDFReportService - Remove unused 'now' variable in ExportDialog - Remove unused 'trackBackspaceAction' import in test - Simplify PDF service test to remove unused 'testData'
…CommunicationHistory component and tests
…BoardContainer component
… formatting and improved readability
There was a problem hiding this comment.
Pull request overview
This pull request addresses missing tracking functionality in the Communication History feature (originally implemented in PR #1987) and fixes a PDF font configuration issue. The changes add tracking for phrase spoken events, clear actions, and backspace events that were previously not being captured.
Changes:
- Integrates tracking actions (trackPhraseSpoken, trackClearAction, trackBackspaceAction) into the Output container to capture user interactions when playing output, clearing, or using backspace
- Integrates trackSymbolSelection into the Board container to capture symbol tile clicks
- Adds Communication History reducer to Redux store configuration
- Creates new Settings page for Communication History with statistics display and PDF export functionality
- Implements comprehensive PDF report generation service with proper font configuration using existing FONTS constant
- Adds extensive test coverage for actions, reducer, and integration scenarios
- Includes detailed documentation in README explaining feature usage and architecture
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/PDFReportService.js | New service for generating PDF reports with proper font configuration using FONTS constant from Export.constants |
| src/reducers.js | Registers communicationHistory reducer in Redux store |
| src/components/Settings/Settings.wrapper.js | Adds routing for Communication History settings page |
| src/components/Settings/Settings.messages.js | Adds i18n message key for Communication History menu item |
| src/components/Settings/Settings.component.js | Adds Communication History menu item with Assessment icon |
| src/components/Settings/CommunicationHistory/* | New Settings page components for displaying statistics and exporting reports |
| src/components/CommunicationHistory/* | Core communication history components including actions, constants, reducer, and export dialog |
| src/components/Board/Output/Output.container.js | Integrates tracking for phrase spoken, clear, and backspace actions |
| src/components/Board/Board.container.js | Integrates tracking for symbol selection with enhanced tile metadata |
| metadata: { | ||
| symbolCount: output.length, | ||
| hasImages: output.some((s) => s.image), | ||
| }, |
There was a problem hiding this comment.
The trackPhraseSpoken function doesn't capture vocalization metadata. The metadata object should include a vocalization field that represents what was actually spoken (concatenated vocalization or label fields from symbols), similar to how trackSymbolSelection includes vocalization in its metadata. Consider adding: metadata.vocalization = output.map((s) => s.vocalization || s.label || '').join(' ').
| import { | ||
| addCommunicationEntry, | ||
| trackSymbolSelection, | ||
| trackPhraseSpoken, | ||
| trackClearAction, | ||
| } from './CommunicationHistory.actions'; |
There was a problem hiding this comment.
The test file imports trackBackspaceAction in the integration test at line 173 but doesn't explicitly test it as a standalone action (like trackSymbolSelection and trackPhraseSpoken are tested separately). Consider adding a dedicated test case for trackBackspaceAction to maintain test coverage consistency.
| const { cancelSpeech, trackBackspaceAction } = this.props; | ||
| cancelSpeech(); | ||
| trackBackspaceAction(); |
There was a problem hiding this comment.
The trackBackspaceAction is being called without userId and sessionId parameters. These parameters should be passed to ensure proper tracking and filtering of communication history per user and session. Similar to the trackPhraseSpoken issue, userData and sessionId should be added to mapStateToProps and then passed to this tracking function.
| const { cancelSpeech, trackBackspaceAction } = this.props; | |
| cancelSpeech(); | |
| trackBackspaceAction(); | |
| const { cancelSpeech, trackBackspaceAction, userData, sessionId } = this.props; | |
| cancelSpeech(); | |
| trackBackspaceAction(userData && userData.id, sessionId); |
| const { cancelSpeech, trackClearAction } = this.props; | ||
| cancelSpeech(); | ||
| trackClearAction(); |
There was a problem hiding this comment.
The trackClearAction is being called without userId and sessionId parameters. These parameters should be passed to ensure proper tracking and filtering of communication history per user and session. Similar to the trackPhraseSpoken issue, userData and sessionId should be added to mapStateToProps and then passed to this tracking function.
| const { cancelSpeech, trackClearAction } = this.props; | |
| cancelSpeech(); | |
| trackClearAction(); | |
| const { cancelSpeech, trackClearAction, userData, sessionId } = this.props; | |
| cancelSpeech(); | |
| trackClearAction(userData, sessionId); |
| ) | ||
| label: PropTypes.oneOfType([PropTypes.string, PropTypes.node]), | ||
| }), | ||
| ), |
There was a problem hiding this comment.
The PropTypes definition is missing validations for the new tracking action props (trackPhraseSpoken, trackClearAction, trackBackspaceAction) as well as userData and sessionId. These should be added to maintain type safety and documentation consistency with the rest of the codebase.
| ), | |
| ), | |
| /** | |
| * Tracking actions | |
| */ | |
| trackPhraseSpoken: PropTypes.func.isRequired, | |
| trackClearAction: PropTypes.func.isRequired, | |
| trackBackspaceAction: PropTypes.func.isRequired, | |
| /** | |
| * User/session metadata | |
| */ | |
| userData: PropTypes.object, | |
| sessionId: PropTypes.string, |
| disableTour: PropTypes.func, | ||
| isLiveMode: PropTypes.bool, | ||
| changeDefaultBoard: PropTypes.func | ||
| changeDefaultBoard: PropTypes.func, |
There was a problem hiding this comment.
The PropTypes definition is missing validations for the new tracking action props (trackSymbolSelection, trackPhraseSpoken, trackClearAction, trackBackspaceAction). These should be added to maintain type safety and documentation consistency with the rest of the codebase.
| changeDefaultBoard: PropTypes.func, | |
| changeDefaultBoard: PropTypes.func, | |
| trackSymbolSelection: PropTypes.func, | |
| trackPhraseSpoken: PropTypes.func, | |
| trackClearAction: PropTypes.func, | |
| trackBackspaceAction: PropTypes.func, |
| trackSymbolSelection( | ||
| enhancedTile, | ||
| userData?.email || userData?.id || null, | ||
| this.props.sessionId || null, |
There was a problem hiding this comment.
The sessionId is being accessed via this.props.sessionId but it's not defined in mapStateToProps. Either add sessionId to mapStateToProps if it exists in the Redux state, or generate/manage it at the component level to ensure consistent session tracking across user interactions.
| trackSymbolSelection( | |
| enhancedTile, | |
| userData?.email || userData?.id || null, | |
| this.props.sessionId || null, | |
| const sessionId = | |
| this.props.sessionId || this.sessionId || shortid.generate(); | |
| if (!this.sessionId) { | |
| this.sessionId = sessionId; | |
| } | |
| trackSymbolSelection( | |
| enhancedTile, | |
| userData?.email || userData?.id || null, | |
| sessionId, |
| const { output, trackPhraseSpoken } = this.props; | ||
|
|
||
| if (liveText) { | ||
| await this.speakOutput(liveText); | ||
| } else { | ||
| if (output.length > 0) { | ||
| trackPhraseSpoken(output); |
There was a problem hiding this comment.
The tracking actions (trackPhraseSpoken, trackClearAction, trackBackspaceAction) are being called without userId and sessionId parameters. These parameters should be passed to ensure proper tracking and filtering of communication history per user and session. The userData and sessionId should be added to mapStateToProps and then passed to these tracking functions.
| const { output, trackPhraseSpoken } = this.props; | |
| if (liveText) { | |
| await this.speakOutput(liveText); | |
| } else { | |
| if (output.length > 0) { | |
| trackPhraseSpoken(output); | |
| const { output, trackPhraseSpoken, userData, sessionId } = this.props; | |
| if (liveText) { | |
| await this.speakOutput(liveText); | |
| } else { | |
| if (output.length > 0) { | |
| trackPhraseSpoken(output, userData, sessionId); |
| isLiveMode: board.isLiveMode, | ||
| navigationSettings: app.navigationSettings, | ||
| increaseOutputButtons: app.displaySettings.increaseOutputButtons | ||
| increaseOutputButtons: app.displaySettings.increaseOutputButtons, |
There was a problem hiding this comment.
The mapStateToProps is missing userData and sessionId which are needed for the tracking actions. Add these to ensure proper tracking: userData from app.userData and consider adding sessionId if it's stored in state, or generate one at the component level.
| increaseOutputButtons: app.displaySettings.increaseOutputButtons, | |
| increaseOutputButtons: app.displaySettings.increaseOutputButtons, | |
| userData: app.userData, | |
| sessionId: app.sessionId, |
…ve prop validation
…mmunication entry
…on-history-pdf-report
- Keep tracking actions (trackSymbolSelection, trackPhraseSpoken, trackClearAction, trackBackspaceAction) and add showPremiumRequired in Board.container.js mapDispatchToProps - Keep communicationHistoryReducer import in reducers.js Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes in the communication history tracking functionality that was partially implemented in PR #1987. The original implementation only tracked symbol selections but was missing tracking for phrase spoken, clear action, and backspace events.
Issues Fixed
As noted in the #1987 (comment), the following tracking actions were not being called:
Additionally fixed:
Testing
Fixes #1987
Closes #1978