[#10950] Track student feedback session in frontend #11005

AdithyaNarayan · 2021-03-04T08:10:51Z

Outline of solution

Added new log service in frontend and call the create logs API in the session submission page for access and submission
Added the log types in teammates.ui.constants so that the type is generated for frontend use (and extensibility in case more log types apart from feedback session in future?)

t-cheepeng · 2021-03-04T09:21:24Z

src/web/app/pages-session/session-submission-page/session-submission-page.component.ts

@@ -310,6 +313,16 @@ export class SessionSubmissionPageComponent implements OnInit, AfterViewInit {
          }
        }

+        this.logService.createFeedbackSessionLog({
+          courseId: this.courseId,
+          studentEmail: this.loggedInUser,


Let's log down the fsname too

Also, are you sure this is the best place to track the student access? What if the user is using a regKey?

Wouldn't the loadFeedbackSession function be called regardless of which auth the user uses?

It is true that the method will be called if auth succeeds. But the thing is the loggedInUser may not contain the email of the student. I was thinking a safer place to put it would be in the method loadPersonName

t-cheepeng · 2021-03-04T09:38:26Z

src/web/app/pages-session/session-submission-page/session-submission-page.component.ts

+        }).subscribe(() => {
+
+        }, () => {
+          this.simpleModalService.openInformationModal('Log Error', SimpleModalType.WARNING, '');


Maybe would be better to use a toast instead?

@damithc If the tracking of student access/submission fails, would a modal or toast be more appropriate?

changed to toast. I feel it makes more sense too. thanks for the feedback

t-cheepeng · 2021-03-04T09:38:42Z

src/web/services/log.service.ts

+   */
+  createFeedbackSessionLog(queryParams: {
+    courseId: string,
+    studentEmail: string,


same here. log down the fsname too

I was also thinking of logging the fsname but the current API endpoint only takes in those 3, right? The labels are only setup for the 3 params. Should I add the new parameter now itself?

For the new API endpoint functionality as in #10984, I've recently changed it to take in fsname too! Sorry if it's a source of confusion.

halfwhole · 2021-03-04T14:11:58Z

src/web/services/log.service.ts

+   */
+  createFeedbackSessionLog(queryParams: {
+    courseId: string,
+    studentEmail: string,


For the new API endpoint functionality as in #10984, I've recently changed it to take in fsname too! Sorry if it's a source of confusion.

halfwhole · 2021-03-04T14:16:22Z

src/web/services/log.service.ts

+    logType: LogTypes }): Observable<string> {
+    const paramMap: Record<string, string> = {
+      courseId: queryParams.courseId,
+      studentEmail: queryParams.studentEmail,


courseid and studentemail should be in lowercase here

Fixed. Thanks

halfwhole

LGTM, works as expected. Might want to eventually write more tests for the log service though

halfwhole · 2021-03-13T07:36:25Z

src/main/java/teammates/ui/constants/LogTypes.java

+/**
+ * Constant values for all the different log types.
+ */
+public enum LogTypes {


Sorry, just a nit: could this be called LogType instead?

rrtheonlyone

I am okay with this change - again hard to give a proper review since it feels like this is just one piece of the puzzle.

* Add tracking to frontend * Fix test case for log service * Fix lint * Add fsname to params * Shift log creation to loadPersonName Co-authored-by: Adithya Narayan Rangarajan Sreenivasan <e0426343@u.nus.edu>

Add tracking to frontend

a1907cb

AdithyaNarayan requested review from t-cheepeng and halfwhole March 4, 2021 08:14

AdithyaNarayan added the s.ToReview The PR is waiting for review(s) label Mar 4, 2021

Adithya Narayan Rangarajan Sreenivasan added 2 commits March 4, 2021 17:05

Fix test case for log service

4242f23

Fix lint

200fc36

t-cheepeng requested changes Mar 4, 2021

View reviewed changes

halfwhole reviewed Mar 4, 2021

View reviewed changes

Adithya Narayan Rangarajan Sreenivasan added 2 commits March 6, 2021 23:17

Add fsname to params

d67d73d

Shift log creation to loadPersonName

a28e9a5

AdithyaNarayan mentioned this pull request Mar 10, 2021

[#10950] Add base files for audit logs backend #10978

Merged

halfwhole approved these changes Mar 13, 2021

View reviewed changes

halfwhole reviewed Mar 13, 2021

View reviewed changes

t-cheepeng requested review from madanalogy, ChooJeremy and rrtheonlyone March 14, 2021 08:26

rrtheonlyone approved these changes Mar 14, 2021

View reviewed changes

rrtheonlyone merged commit 76fcd8e into TEAMMATES:audit-logs Mar 14, 2021

halfwhole mentioned this pull request Mar 18, 2021

[#10950] Add logging functionality to backend #10984

Merged

madanalogy added this to In progress in Audit Logs for Instructors via automation Mar 22, 2021

madanalogy moved this from In progress to Done in Audit Logs for Instructors Mar 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#10950] Track student feedback session in frontend #11005

[#10950] Track student feedback session in frontend #11005

AdithyaNarayan commented Mar 4, 2021

t-cheepeng Mar 4, 2021

t-cheepeng Mar 4, 2021

AdithyaNarayan Mar 6, 2021

t-cheepeng Mar 8, 2021

t-cheepeng Mar 4, 2021

AdithyaNarayan Mar 6, 2021

t-cheepeng Mar 4, 2021

AdithyaNarayan Mar 4, 2021

halfwhole Mar 4, 2021

halfwhole Mar 4, 2021

halfwhole Mar 4, 2021

AdithyaNarayan Mar 6, 2021

halfwhole left a comment

halfwhole Mar 13, 2021

rrtheonlyone left a comment

[#10950] Track student feedback session in frontend #11005

[#10950] Track student feedback session in frontend #11005

Conversation

AdithyaNarayan commented Mar 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

halfwhole left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rrtheonlyone left a comment

Choose a reason for hiding this comment