-
Notifications
You must be signed in to change notification settings - Fork 357
Development: Add additional event log types for research purposes
#10735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development: Add additional event log types for research purposes
#10735
Conversation
WalkthroughThis change extends the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AngularComponent
participant ScienceService
User->>AngularComponent: Interact with competency or learning path UI
AngularComponent->>ScienceService: logEvent(eventType, resourceId)
Suggested labels
Suggested reviewers
✨ Finishing Touches
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. 🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.ts (1)
51-56: Consider consolidating conditional logic.While the current implementation works correctly, consider placing both the resource ID setting and event logging inside the same conditional block for clarity.
// log event if (learningPath) { this.setResourceId(learningPath.id); + this.logEvent(); } -this.logEvent();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/de/tum/cit/aet/artemis/atlas/domain/science/ScienceEventType.java(1 hunks)src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.ts(3 hunks)src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.ts(4 hunks)src/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.ts(2 hunks)src/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.ts(3 hunks)src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.ts(3 hunks)src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.ts(4 hunks)src/main/webapp/app/shared/science/science.component.ts(1 hunks)src/main/webapp/app/shared/science/science.model.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.tssrc/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.tssrc/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.tssrc/main/webapp/app/shared/science/science.component.tssrc/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.tssrc/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.tssrc/main/webapp/app/shared/science/science.model.tssrc/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.ts
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/atlas/domain/science/ScienceEventType.java
🧬 Code Graph Analysis (1)
src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.ts (1)
src/main/webapp/app/atlas/shared/entities/learning-path.model.ts (1)
LearningPathNavigationObjectDTO(38-46)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: server-tests
- GitHub Check: client-tests-selected
- GitHub Check: client-tests
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Analyse
🔇 Additional comments (29)
src/main/java/de/tum/cit/aet/artemis/atlas/domain/science/ScienceEventType.java (1)
9-10: New event types added following naming convention.The additions for competency and learning path event types follow the documented naming convention of
<category>__<detailed event name>. These new constants align well with the PR objective of tracking interactions with competencies and learning paths for research purposes.src/main/webapp/app/shared/science/science.component.ts (3)
10-10: Event type parameter made optional.Making the
typeparameter optional allows for more flexible initialization patterns in derived components.
14-16: New method enhances flexibility.The
setScienceEventTypemethod allows components to set the event type dynamically after construction, supporting more complex use cases where the event type isn't known at initialization time.
22-25: Defensive programming applied to prevent errors.Adding a conditional check before calling
scienceService.logEventprevents potential runtime errors when no event type is defined. This is a good practice that makes the component more robust.src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.ts (3)
14-15: Imports added for science event tracking.The necessary imports are added to support the new event tracking functionality.
24-24: Component extended to support science event tracking.The component now extends
AbstractScienceComponentto enable tracking of learning path interactions, aligning with the PR objectives.
39-39: Event type specified in constructor.The component passes
ScienceEventType.LEARNING_PATH__OPENto the parent constructor, clearly indicating its purpose for tracking when learning paths are opened.src/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.ts (4)
13-14: Imports added for science event tracking.The necessary imports are added to support the new event tracking functionality.
23-23: Component extended to support science event tracking.The component now extends
AbstractScienceComponentto enable tracking of learning path navigation interactions, aligning with the PR objectives.
45-45: Event type specified in constructor.The component passes
ScienceEventType.LEARNING_PATH__OPEN_NAVIGATIONto the parent constructor, clearly indicating its purpose for tracking when learning path navigation is opened.
48-51: Event tracking added to effect.Event tracking is appropriately placed within the effect that reacts to changes in the learning path ID, ensuring events are logged when the navigation component is initialized with a new learning path.
src/main/webapp/app/shared/science/science.model.ts (1)
10-16: Correctly implemented new science event types.The new event types for competency and learning path interactions follow the established naming convention of
<category>__<detailed event name>and align perfectly with the PR objectives of tracking student interactions with competency and learning path UI components.src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.ts (4)
45-46: Good addition of necessary science component imports.The imports for AbstractScienceComponent and ScienceEventType are correctly added to support the event logging functionality.
72-72: Class properly extends AbstractScienceComponent.The component now extends AbstractScienceComponent to inherit the science event logging functionality, which aligns with the PR objective of tracking competency interactions.
98-100: Constructor implementation for science event tracking.The constructor correctly passes the COMPETENCY__OPEN event type to the parent class.
118-123: Science event logging implementation.The resource ID is properly set to the competency ID before logging the event, ensuring accurate tracking of which competency was viewed.
src/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.ts (4)
17-18: Good addition of necessary science component imports.The imports for AbstractScienceComponent and ScienceEventType are correctly added to support the event logging functionality.
26-26: Class properly extends AbstractScienceComponent.The component now extends AbstractScienceComponent to inherit the science event logging functionality, which aligns with the PR objective of tracking competency overview interactions.
51-53: Constructor implementation for science event tracking.The constructor correctly passes the COMPETENCY__OPEN_OVERVIEW event type to the parent class.
61-65: Science event logging implementation.The resource ID is properly set to the course ID before logging the event, ensuring accurate tracking of competency overview interactions within the context of a specific course.
src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.ts (4)
10-11: Good addition of necessary science component imports.The imports for AbstractScienceComponent and ScienceEventType are correctly added to support the event logging functionality.
20-20: Class properly extends AbstractScienceComponent.The component now extends AbstractScienceComponent to inherit the science event logging functionality, which aligns with the PR objective of tracking learning path competency graph interactions.
33-34: Constructor implementation for science event tracking.The constructor correctly passes the LEARNING_PATH__OPEN_GRAPH event type to the parent class.
38-40: Science event logging implementation.The resource ID is properly set to the learning path ID before logging the event. This implementation is placed appropriately within the effect function to ensure it runs when the learning path ID changes.
src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.ts (5)
10-11: Appropriate imports for science event trackingThese imports correctly bring in the necessary components for science event tracking, aligning with the PR's objective of adding research-focused event logging.
20-20: Well-implemented inheritance for science trackingThe component now appropriately extends AbstractScienceComponent, enabling it to log user interactions with learning path navigation. This change follows Angular best practices and the PascalCase naming convention required by the coding guidelines.
45-45: Proper parent constructor callThe call to super() is correctly implemented, which is necessary when extending a parent class.
48-48: Resource ID tracking correctly configuredThe resource ID is properly set using the current learning path ID, ensuring that all logged events can be associated with the specific learning path the user is interacting with.
54-57: Well-implemented navigation event loggingThe implementation correctly:
- Uses a clear comment to indicate the purpose of the code
- Sets the appropriate event type based on navigation direction
- Logs the event before performing the actual navigation
This approach ensures accurate tracking of user navigation patterns while maintaining the component's core functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/lecture/overview/course-lectures/attachment-unit/attachment-unit.component.ts (1)
56-56: Consider using more specific event types for different actions.Both
handleDownload()andhandleOriginalVersion()use the same event type (LECTURE__OPEN_UNIT). For research purposes, it might be beneficial to differentiate between these distinct user interactions with more specific event types.Also applies to: 68-68
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.ts(3 hunks)src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.ts(3 hunks)src/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.ts(3 hunks)src/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.ts(3 hunks)src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.ts(3 hunks)src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.ts(3 hunks)src/main/webapp/app/core/course/overview/exercise-details/course-exercise-details.component.ts(4 hunks)src/main/webapp/app/lecture/overview/course-lectures/attachment-unit/attachment-unit.component.ts(4 hunks)src/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.ts(4 hunks)src/main/webapp/app/lecture/overview/course-lectures/lecture-unit/lecture-unit.directive.ts(1 hunks)src/main/webapp/app/lecture/overview/course-lectures/online-unit/online-unit.component.ts(2 hunks)src/main/webapp/app/lecture/overview/course-lectures/text-unit/text-unit.component.ts(3 hunks)src/main/webapp/app/lecture/overview/course-lectures/video-unit/video-unit.component.ts(2 hunks)src/main/webapp/app/shared/science/science.component.spec.ts(0 hunks)src/main/webapp/app/shared/science/science.component.ts(0 hunks)src/main/webapp/app/shared/science/science.directive.spec.ts(0 hunks)src/main/webapp/app/shared/science/science.directive.ts(0 hunks)
💤 Files with no reviewable changes (4)
- src/main/webapp/app/shared/science/science.directive.ts
- src/main/webapp/app/shared/science/science.component.ts
- src/main/webapp/app/shared/science/science.component.spec.ts
- src/main/webapp/app/shared/science/science.directive.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.ts
- src/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.ts
- src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.ts
- src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.ts
- src/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.ts
- src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.ts
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/lecture/overview/course-lectures/video-unit/video-unit.component.tssrc/main/webapp/app/lecture/overview/course-lectures/attachment-unit/attachment-unit.component.tssrc/main/webapp/app/lecture/overview/course-lectures/online-unit/online-unit.component.tssrc/main/webapp/app/lecture/overview/course-lectures/lecture-unit/lecture-unit.directive.tssrc/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.tssrc/main/webapp/app/core/course/overview/exercise-details/course-exercise-details.component.tssrc/main/webapp/app/lecture/overview/course-lectures/text-unit/text-unit.component.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: Analyse
🔇 Additional comments (11)
src/main/webapp/app/lecture/overview/course-lectures/attachment-unit/attachment-unit.component.ts (3)
25-26: LGTM: Properly injected ScienceService for event logging.The addition of the ScienceService and ScienceEventType imports, along with the proper injection using Angular's recommended inject function, follows good practices for dependency injection.
Also applies to: 37-37
56-56: Event logging added to file download action.The code correctly logs the LECTURE__OPEN_UNIT event when a user downloads a file, providing valuable data for the research objectives mentioned in the PR.
68-68: Event logging added to original version access.The code correctly logs the LECTURE__OPEN_UNIT event when a user accesses the original version of a file.
src/main/webapp/app/lecture/overview/course-lectures/online-unit/online-unit.component.ts (2)
1-1: LGTM: Properly imported and injected ScienceService.The modifications to import the required dependencies and inject the ScienceService follow Angular best practices using the recommended inject function.
Also applies to: 6-7, 17-18
20-20: Event logging added to isolated view action.The code correctly logs the LECTURE__OPEN_UNIT event when a user opens an online resource in an isolated view, which supports the research objectives outlined in the PR.
src/main/webapp/app/lecture/overview/course-lectures/video-unit/video-unit.component.ts (2)
1-1: LGTM: Properly imported and injected ScienceService.The additions to import the required dependencies and inject the ScienceService follow Angular best practices using the recommended inject function.
Also applies to: 8-9, 36-37
41-41: Event logging added to video expansion.The code correctly logs the LECTURE__OPEN_UNIT event when a user expands a video unit, which aligns with the research objectives for tracking student interactions.
src/main/webapp/app/lecture/overview/course-lectures/lecture-unit/lecture-unit.directive.ts (1)
2-2: LGTM: Successfully refactored to remove AbstractScienceComponent dependency.The directive has been properly refactored to remove the inheritance from the now-removed AbstractScienceComponent while maintaining its core functionality. This change aligns with the broader refactoring pattern where components now directly inject ScienceService instead of inheriting from a base class.
Also applies to: 7-7
src/main/webapp/app/core/course/overview/exercise-details/course-exercise-details.component.ts (1)
58-58: Great implementation of the science event logging.The refactoring from
AbstractScienceComponentto directScienceServiceinjection is well implemented. This approach provides better decoupling and simplifies the component hierarchy.Also applies to: 110-110, 184-184
src/main/webapp/app/lecture/overview/course-lectures/text-unit/text-unit.component.ts (1)
8-9: Clean implementation of science event logging.The direct service injection approach maintains clean separation of concerns and removes the dependency on an abstract component. This is a good architectural improvement.
Also applies to: 20-20, 30-30
src/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.ts (1)
39-39: Consistent implementation of science event logging.The refactoring consistently applies the same pattern as other components, ensuring a uniform approach to event logging throughout the application. This change aligns perfectly with the PR's goal of adding event logging for research purposes.
Also applies to: 77-77, 109-109
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
raffifasaro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
441abcd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.spec.ts (1)
40-90: Consider adding a test for the new event logging behavior.While the mock service is properly configured, there's no test specifically verifying that the component logs the expected COMPETENCY__OPEN event during initialization. Adding a test to verify this new behavior would improve test coverage.
You could add a test like this:
it('should log COMPETENCY__OPEN event on initialization', () => { const scienceService = TestBed.inject(ScienceService); const logEventSpy = jest.spyOn(scienceService, 'logEvent'); fixture.detectChanges(); expect(logEventSpy).toHaveBeenCalledWith(ScienceEventType.COMPETENCY__OPEN, expect.any(Object)); });This would require importing ScienceEventType as well:
import { ScienceService, ScienceEventType } from 'app/shared/science/science.service';
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.spec.ts(2 hunks)src/main/webapp/app/atlas/manage/learning-paths-table/learning-paths-table.component.spec.ts(2 hunks)src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.spec.ts(2 hunks)src/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.spec.ts(2 hunks)src/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.spec.ts(2 hunks)src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.spec.ts(2 hunks)src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.spec.ts(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/main/webapp/app/atlas/overview/learning-path-nav-overview/learning-path-nav-overview.component.spec.ts
- src/main/webapp/app/atlas/overview/course-competencies/course-competencies.component.spec.ts
- src/main/webapp/app/atlas/manage/competency-graph-modal/competency-graph-modal.component.spec.ts
- src/main/webapp/app/atlas/manage/learning-paths-table/learning-paths-table.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.spec.tssrc/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.spec.tssrc/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-tests-selected
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (3)
src/main/webapp/app/atlas/overview/learning-path-student-nav/learning-path-student-nav.component.spec.ts (1)
8-9: Testing setup correctly updated for ScienceService integration.The changes properly update the test configuration to support the new ScienceService dependency. Adding MockProvider(ScienceService) to the providers array ensures that tests will continue to run correctly with the new event logging functionality, without requiring actual service implementation.
Also applies to: 69-69
src/main/webapp/app/atlas/overview/learning-path-student-page/learning-path-student-page.component.spec.ts (1)
17-18: Well-implemented test dependency update.The addition of
ScienceServicemock provider correctly supports the component's new direct dependency on this service, which aligns with the PR objective of adding event logging for learning paths. UsingMockProviderfrom ng-mocks is an efficient approach that follows best practices for Angular testing.Also applies to: 59-59
src/main/webapp/app/atlas/overview/course-competencies/course-competencies-details.component.spec.ts (1)
40-40: Good addition of ScienceService support.The import and mock provider for ScienceService are correctly implemented, which aligns with the PR objective of adding event logging for competency interactions. This properly supports the corresponding component change that now injects this service to log a COMPETENCY__OPEN event during initialization.
Also applies to: 90-90
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
raffifasaro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-approve
Checklist
General
Server
Client
Motivation and Context
For an upcoming evaluation we want to analyze behavioral data of students. Our goal is to investigate how effective our instantiation of competency-based education is.
Description
Added new event log entries for Interactions with:
Steps for Testing
Prerequisites:
science_eventshould contain an entry for viewing the competency overview)science_eventshould contain an entry for viewing the competency)science_eventshould contain an entry for viewing the learning path)science_eventshould contain an entry for viewing the next task)science_eventshould contain an entry for viewing the previous task)science_eventshould contain an entry for viewing the navigation)science_eventshould contain an entry for viewing the competency graph)Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
unchanged
Summary by CodeRabbit
New Features
Refactor
Chores