[#10969] Search form for audit logs page #11006

t-cheepeng · 2021-03-04T09:43:23Z

Outline of solution

Added a search form to audit logs page as per wireframe in Instructor Audit Logs Frontend #10969

Current Page

halfwhole · 2021-03-07T03:07:42Z

...eb/app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.html

+      <div class="col-5">
+        <div class="col-12">
+          <label for="course-id-dropdown">Course ID</label>
+        </div>
+        <div class="col-12">
+          <select id="course-id-dropdown" class="form-control col-6" [(ngModel)]="formModel.courseId">
+            <option *ngFor="let course of courses">
+              {{course.courseId}}
+            </option>
+          </select>
+        </div>
+      </div>
+      <div class="col-6">
+        <div class="col-12">
+          <label for="student-name-input">Student Name (Optional)</label>
+        </div>
+        <div class="col-12">
+          <input id="student-name-input" class="form-control col-6" type="text" [(ngModel)]="formModel.studentName" placeholder="Student Name"/>
+        </div>
+      </div>
+      <div class="col-1">
+        <button class="btn btn-primary mt-4" (click)="search()">Search</button>
+      </div>


Is there a way to align these with the row above? Currently the 5/6/1 column split in the bottom row looks a bit off compared to the 6/6 in the top row

halfwhole

LGTM

AdithyaNarayan · 2021-03-10T12:05:43Z

Do we need to also add a Feedback Session Name dropdown that is generated after the Course ID is chosen?

t-cheepeng · 2021-03-10T12:09:54Z

Do we need to also add a Feedback Session Name dropdown that is generated after the Course ID is chosen?

I think the plan is to fetch the logs for the whole of the courseid. possibly include the advanced filtering in a future PR

AdithyaNarayan

LGTM

madanalogy

UI looks good, just some comments below

madanalogy · 2021-03-11T14:43:44Z

...eb/app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.html

@@ -0,0 +1,73 @@
+<h1>Student Activity Logs</h1>
+<p>This page allows you to find when your students have accessed or submitted a particular feedback session.</p>


Are there plans to include filtering based on feedback session after selecting / filling in the course ID? If yes, then you might want to include a skeleton template for that. If no, then maybe reword this prompt to include "for a given course"

Will reword it

madanalogy · 2021-03-11T14:44:17Z

src/web/app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.ts

+   * Search for logs of student activity
+   */
+  search(): void {
+    // TODO: Call endpoint to retrieve logs


Might be more worthwhile to have this PR on hold until the API endpoint and data formats are finalised

The endpoint return type is specified in #10978 but should not really matter for this PR since the PR only deals with the search form for now

madanalogy · 2021-03-11T14:46:00Z

...eb/app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.html

+        </div>
+        <div class="col-12">
+          <select id="course-id-dropdown" class="form-control col-6" [(ngModel)]="formModel.courseId">
+            <option *ngFor="let course of courses">


Not sure how this would work out for instructors with lots of courses. Any possibility of having the courses sorted by date or allowing filtering based on text input? Or is that already a built in function of the select field?

How about sort by alphabetical order? I would think that most instructors don't really have that many courses that the dropdown will impede the UX?

I would think that most instructors don't really have that many courses that the dropdown will impede the UX?

Best to check UI/UX matters with @damithc

damithc · 2021-03-13T07:49:43Z

Regrading the UI:
Better term for Logs from? e.g., Search period: from, until
The drop-down for the time is too wide?
I'm interested to see how the search results will be displayed.

On a separate note, is the backend capable of searching the full period within the 1-minute window given to each request?

t-cheepeng · 2021-03-14T08:19:00Z

@damithc
Have shortened the time picker and bolded/changed the form names. Is the search form okay?

I'm interested to see how the search results will be displayed.

That should be coming in a separate PR from this, but shouldn't look too dissimilar from the wireframe

On a separate note, is the backend capable of searching the full period within the 1-minute window given to each request?

From some preliminary testing on the staging server, retrieving 2000 log entries without filtering (about 2 days worth) from request logs took about 1 minute. With more conditions on the retrieval, the period of time searched for can be longer. But it is unlikely that we are able to stay below 1 minute beyond 5 days worth of logs (from a rough estimate). Will need more testing on this

damithc · 2021-03-14T08:33:08Z

Have shortened the time picker and bolded/changed the form names. Is the search form okay?

It's better now. Note that layout (i.e., alignment, spacing) can be used to convey additional information about the meaning of the fields.

The current layout suggests two main groups of inputs but that grouping is not aligned with the actual meaning of the inputs.
Also, the submit button is a bit out of place.

From some preliminary testing on the staging server, retrieving 2000 log entries without filtering (about 2 days worth) from request logs took about 1 minute. With more conditions on the retrieval, the period of time searched for can be longer. But it is unlikely that we are able to stay below 1 minute beyond 5 days worth of logs (from a rough estimate). Will need more testing on this

Got it. That means the output could be up to a certain end time which is earlier than the actual end time specified by the user. The UI for the results should indicate that clearly.

t-cheepeng · 2021-03-15T03:25:08Z

@damithc
Two different groupings shown below. Which one would be clearer?

Grouping 1:

Grouping 2:

damithc · 2021-03-15T15:55:26Z

Two different groupings shown below. Which one would be clearer?

Of these two, I prefer the latter. You can also get some ideas from the form used to create a session.

If you think it is more natural for the user to choose the course first, that should be on the left half.
Is the user expected to enter the student name exactly as it is in the database? that's not very realistic. Perhaps it should be a dropdown instead?

t-cheepeng · 2021-03-16T09:22:44Z

@damithc Swapped the course id and student to the left side. definitely makes more sense there. Also, the student is a dropdown now

damithc · 2021-03-16T11:57:01Z

@damithc Swapped the course id and student to the left side. definitely makes more sense there. Also, the student is a dropdown now

Looks good enough to me. 👍

rrtheonlyone

LGTM 👍

* Add routing to audit logs page * Add search form for audit logs page * Restructure form for search button * Reword message and add sort for courses * Restructure form and change input to dropdown

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

t-cheepeng requested review from AdithyaNarayan and halfwhole March 4, 2021 09:43

halfwhole reviewed Mar 7, 2021

View reviewed changes

t-cheepeng requested a review from halfwhole March 8, 2021 05:01

halfwhole approved these changes Mar 10, 2021

View reviewed changes

t-cheepeng requested review from ChooJeremy, madanalogy and rrtheonlyone March 10, 2021 12:10

AdithyaNarayan approved these changes Mar 10, 2021

View reviewed changes

madanalogy reviewed Mar 11, 2021

View reviewed changes

t-cheepeng requested a review from madanalogy March 12, 2021 05:59

t-cheepeng added 5 commits March 16, 2021 17:21

Add routing to audit logs page

fcd2aa6

Add search form for audit logs page

3d91aff

Restructure form for search button

f929407

Reword message and add sort for courses

74047ff

Restructure form and change input to dropdown

1b5c5ba

t-cheepeng force-pushed the 10969-skeleton-frontend-page branch from 68200b9 to 1b5c5ba Compare March 16, 2021 09:23

t-cheepeng requested review from madanalogy and removed request for madanalogy March 19, 2021 03:57

rrtheonlyone approved these changes Mar 21, 2021

View reviewed changes

rrtheonlyone merged commit cfedb58 into TEAMMATES:audit-logs Mar 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#10969] Search form for audit logs page #11006

[#10969] Search form for audit logs page #11006

t-cheepeng commented Mar 4, 2021 •

edited

halfwhole Mar 7, 2021

halfwhole left a comment

AdithyaNarayan commented Mar 10, 2021

t-cheepeng commented Mar 10, 2021

AdithyaNarayan left a comment

madanalogy left a comment

madanalogy Mar 11, 2021

t-cheepeng Mar 12, 2021

madanalogy Mar 11, 2021

t-cheepeng Mar 12, 2021

madanalogy Mar 11, 2021

t-cheepeng Mar 12, 2021

madanalogy Mar 12, 2021

damithc commented Mar 13, 2021

t-cheepeng commented Mar 14, 2021

damithc commented Mar 14, 2021

t-cheepeng commented Mar 15, 2021

damithc commented Mar 15, 2021

t-cheepeng commented Mar 16, 2021

damithc commented Mar 16, 2021

rrtheonlyone left a comment

		@@ -0,0 +1,73 @@
		<h1>Student Activity Logs</h1>
		<p>This page allows you to find when your students have accessed or submitted a particular feedback session.</p>

[#10969] Search form for audit logs page #11006

[#10969] Search form for audit logs page #11006

Conversation

t-cheepeng commented Mar 4, 2021 • edited

Choose a reason for hiding this comment

halfwhole left a comment

Choose a reason for hiding this comment

AdithyaNarayan commented Mar 10, 2021

t-cheepeng commented Mar 10, 2021

AdithyaNarayan left a comment

Choose a reason for hiding this comment

madanalogy left a 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

damithc commented Mar 13, 2021

t-cheepeng commented Mar 14, 2021

damithc commented Mar 14, 2021

t-cheepeng commented Mar 15, 2021

damithc commented Mar 15, 2021

t-cheepeng commented Mar 16, 2021

damithc commented Mar 16, 2021

rrtheonlyone left a comment

Choose a reason for hiding this comment

t-cheepeng commented Mar 4, 2021 •

edited