Skip to content

[#11126] Expand empty search results for feedback session logs#11151

Merged
rrtheonlyone merged 9 commits into
TEAMMATES:masterfrom
hoonti06:11126-instructor-checking-logs-expand-results-if-empty
Jun 5, 2021
Merged

[#11126] Expand empty search results for feedback session logs#11151
rrtheonlyone merged 9 commits into
TEAMMATES:masterfrom
hoonti06:11126-instructor-checking-logs-expand-results-if-empty

Conversation

@hoonti06

@hoonti06 hoonti06 commented May 24, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #11126

Outline of Solution

  • Modified that tap expand status depends on whether feedback session log entry is empty or not

before
before

after
after

@moziliar moziliar added the s.ToReview The PR is waiting for review(s) label May 25, 2021
@moziliar

moziliar commented May 25, 2021

Copy link
Copy Markdown
Contributor

Hi @hoonti06 can you also attach the before-after snapshot for the ease of review?

@hoonti06

hoonti06 commented May 25, 2021

Copy link
Copy Markdown
Contributor Author

Hi @hoonti06 can you also attach the before-after snapshot for the ease of review?

Sure, I added gif files to compare before and after

moziliar
moziliar approved these changes May 26, 2021
View reviewed changes

@moziliar moziliar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me the change solves the problem.

Need @t-cheepeng to verify if there's any pitfall around this change.

t-cheepeng
t-cheepeng approved these changes May 27, 2021
View reviewed changes

@t-cheepeng t-cheepeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@t-cheepeng t-cheepeng added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels May 27, 2021
@madanalogy madanalogy added this to the V7.17.0 milestone May 28, 2021
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label May 28, 2021
@moziliar

moziliar commented May 28, 2021

Copy link
Copy Markdown
Contributor

@hoonti06 the e2e test case is bind to the old UI. Please update the e2e test case

@madanalogy madanalogy added s.Ongoing The PR is being worked on by the author(s) and removed c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging labels May 28, 2021
@madanalogy madanalogy removed this from the V7.17.0 milestone May 28, 2021
@rrtheonlyone

rrtheonlyone commented May 29, 2021

Copy link
Copy Markdown
Contributor

Thanks for the work here @hoonti06 💯

Looks like E2E tests for Audit Logs are failing - could you take a look and see if you can fix? Let us know if you need any assistance on this.

@moziliar

moziliar commented May 31, 2021

Copy link
Copy Markdown
Contributor

Do try to run e2e test locally before pushing. It will save the time on remote CI and also allow us to get back to you faster, granted that we now limit the CI run for first PR to manual approval:)

@hoonti06

hoonti06 commented Jun 2, 2021

Copy link
Copy Markdown
Contributor Author

Sorry, I just succeeded in configuring my test environment locally.

rrtheonlyone
rrtheonlyone approved these changes Jun 5, 2021
View reviewed changes

@rrtheonlyone rrtheonlyone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@rrtheonlyone rrtheonlyone added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.Ongoing The PR is being worked on by the author(s) labels Jun 5, 2021
@rrtheonlyone rrtheonlyone added this to the V7.17.0 milestone Jun 5, 2021
@rrtheonlyone rrtheonlyone merged commit 1123653 into TEAMMATES:master Jun 5, 2021
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rrtheonlyone rrtheonlyone rrtheonlyone approved these changes

@moziliar moziliar moziliar approved these changes

@t-cheepeng t-cheepeng t-cheepeng approved these changes

Assignees

@rrtheonlyone rrtheonlyone

@moziliar moziliar

@t-cheepeng t-cheepeng

Labels

c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging

Projects

None yet

Milestone

V7.17.0

Development

Successfully merging this pull request may close these issues.

Instructor checking logs: expand results if empty

5 participants

@hoonti06 @moziliar @rrtheonlyone @t-cheepeng @madanalogy