Fix part of #19570: Adding time range filter #19979

masterboy376 · 2024-03-17T17:42:17Z

Overview

This PR fixes part of Fix key release blockers of the contributor admin dashboard #19570.
This PR does the following: Adds a filter to contributor admin dashboard that enable users to fetch reviewer and submitter stats for translation and questions from last selected dates to today.

Essential Checklist

The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
I have followed the instructions for making a code change.
I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I don't have permissions to assign reviewers directly).
The linter/Karma presubmit checks pass on my local machine, and my PR follows the coding style guide).
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)

Proof that changes are correct

screen-recorder-thu-may-16-2024-09-09-36.webm

Proof of changes on desktop with slow/throttled network

screen-recorder-thu-may-16-2024-09-11-32.webm

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

…e-filter

oppiabot · 2024-03-17T17:42:29Z

Hi @masterboy376 please assign the required reviewer(s) for this PR. Thanks!

…e-filter

oppiabot · 2024-04-05T09:21:44Z

Hi @masterboy376, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 4 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 4 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppiabot · 2024-05-28T02:18:11Z

Unassigning @masterboy376 since a re-review was requested. @masterboy376, please make sure you have addressed all review comments. Thanks!

…e-filter

nikitaevg · 2024-05-30T08:13:51Z

...butor-dashboard-admin-page/services/contributor-dashboard-admin-stats-backend-api.service.ts

@@ -47,6 +47,7 @@ export interface TranslationSubmitterBackendDict {
  rejected_translations_count: number;
  rejected_translation_word_count: number;
  first_contribution_date: string;
+  first_contributed_in_days: number;


Where do you use this field?

removed it. actually it was used in the previous implementation.

nikitaevg · 2024-05-30T08:22:28Z

core/controllers/contributor_dashboard_admin_test.py

@@ -845,7 +846,7 @@ def setUp(self) -> None:
            rejected_translations_count=self.REJECTED_TRANSLATIONS_COUNT,
            rejected_translation_word_count=(
                self.REJECTED_TRANSLATION_WORD_COUNT),
-            first_contribution_date=datetime.datetime.utcnow(),


Please remove any unnecessary change from this PR including this. You can add a follow up PR with only these changes and it will be approved smoothly

nikitaevg · 2024-05-30T08:24:15Z

core/domain/suggestion_registry.py

@@ -2097,8 +2098,10 @@ def to_frontend_dict(
                self.rejected_translation_word_count),
            'first_contribution_date': (
                self.first_contribution_date.strftime('%b %d, %Y')),
-            'last_contributed_in_days': int(
-                (datetime.date.today() - self.last_contribution_date).days)
+            'first_contributed_in_days': utils.get_number_of_days_since_date(


IIUC these changes are not needed in this PR. If I missed it and this field is used, please extract it into a separate PR. Ideally this PR should not touch anything on the backend side.

removed the unnecessary code from the PR.

nikitaevg · 2024-05-30T08:26:12Z

...lates/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.html

+              <b *ngIf="activeTab === TAB_NAME_TRANSLATION_SUBMITTER || activeTab === TAB_NAME_QUESTION_SUBMITTER" class="filter-label">Filter by Last Contributed after:</b>
+              <b *ngIf="activeTab === TAB_NAME_TRANSLATION_REVIEWER || activeTab === TAB_NAME_QUESTION_REVIEWER" class="filter-label">Filter by Last Reviewed after:</b>


It's better to put as little logic to html as possible, please move this if-else to TS

oppiabot · 2024-05-30T08:38:22Z

Unassigning @nikitaevg since the review is done.

oppiabot · 2024-05-30T08:38:25Z

Hi @masterboy376, it looks like some changes were requested on this pull request by @nikitaevg. PTAL. Thanks!

masterboy376 · 2024-05-30T10:37:27Z

Hello, @nikitaevg, I have removed all the unnecessary code from the PR to the best of my knowledge PTAL.

oppiabot · 2024-05-30T10:40:31Z

Unassigning @masterboy376 since a re-review was requested. @masterboy376, please make sure you have addressed all review comments. Thanks!

nikitaevg

Much better, thanks

nikitaevg · 2024-05-31T07:54:20Z

...lates/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.html

+              <b *ngIf="isSubmitterTab()" class="filter-label">Filter by Last Contributed after:</b>
+              <b *ngIf="isReviewerTab()" class="filter-label">Filter by Last Reviewed after:</b>
+              <mat-form-field>
+                <input matInput class="e2e-test-last-date-picker-input" [matDatepicker]="lastDatepicker" (dateChange)="changeLastDate($event.value)" [(ngModel)]="lastDateToFilterUsersActivity" [max]="today">


lastDatepicker -> lastDatePicker. Otherwise it seems like a last datepicker, not last date picker

nikitaevg · 2024-05-31T07:55:11Z

...es/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.spec.ts

@@ -422,19 +414,51 @@ describe('Contributor dashboard Admin page', () => {
      expect(component.selectedLanguage.id).toBe(nonDefaultLanguage.id);
    }));

-    it('should select last activity from dropdown', fakeAsync(() => {
+    it(
+      'should initially filters users by whether their last activity occurred ' +


should...filter, not should...filters. Also please check other test names

nikitaevg · 2024-05-31T07:56:12Z

...es/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.spec.ts

+      })
+    );
+
+    it("should changes filter by users' last activity when end date changes", fakeAsync(() => {


This and previous tests could be merged, no need to make them that small

nikitaevg · 2024-05-31T07:58:21Z

...mplates/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.ts

+      this.activeTab === this.TAB_NAME_QUESTION_REVIEWER ||
+      this.activeTab === this.TAB_NAME_QUESTION_REVIEWER


Ah, Thanks for highlighting. Fixed it.

nikitaevg · 2024-05-31T07:58:57Z

...mplates/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.ts

@@ -240,23 +270,46 @@ export class ContributorAdminDashboardPageComponent implements OnInit {
    this.languageDropdownShown = !this.languageDropdownShown;
  }

-  toggleActivityDropdown(): void {
-    this.activityDropdownShown = !this.activityDropdownShown;
+  getDateThatIsDaysBeforeToday(numberOfDaysBeforeToday: number): Date {


Naming here could be improved a bit. Maybe getDateNDaysAgo

nikitaevg · 2024-05-31T07:59:10Z

...mplates/pages/contributor-dashboard-admin-page/contributor-admin-dashboard-page.component.ts

+    );
+  }
+
+  getNumberOfDaysForDateBeforeToday(date: Date): number {


getDaysSince?

oppiabot · 2024-05-31T08:03:06Z

Unassigning @nikitaevg since the review is done.

oppiabot · 2024-05-31T08:03:08Z

Hi @masterboy376, it looks like some changes were requested on this pull request by @nikitaevg. PTAL. Thanks!

…e-filter

masterboy376 · 2024-05-31T09:46:13Z

@nikitaevg, addressed all the latest comments. PTAL.

oppiabot · 2024-05-31T09:49:17Z

Unassigning @masterboy376 since a re-review was requested. @masterboy376, please make sure you have addressed all review comments. Thanks!

…e-filter

oppiabot · 2024-06-02T10:17:18Z

Unassigning @nikitaevg since they have already approved the PR.

masterboy376 added 6 commits March 15, 2024 18:55

created a workingfeature

92a6df1

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

2c9e18c

…e-filter

improved frontend design

385e0bb

convert date to dd-MMM-yyyy format

a73bea1

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

f7bc999

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

af53044

…e-filter

oppiabot bot assigned masterboy376 Mar 17, 2024

masterboy376 added 14 commits March 18, 2024 22:39

backend coverage test passed

3c98fb9

achived full coverage in core/storage/suggestion/gae_model

25446c9

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

e797542

…e-filter

fixing mypy type checks

f5cc817

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

8317a29

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

ef0d15e

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

35a1987

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

9c7d193

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

8b34610

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

6cffbdc

…e-filter

added e2e test

0523ac7

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

018a2a4

…e-filter

modified test

cf478d7

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

8856b42

…e-filter

oppiabot bot added the stale label Apr 5, 2024

masterboy376 added 3 commits April 5, 2024 17:53

updated e2e test

86403f4

some corrections to the test

da567ac

--amend

3c0174b

oppiabot bot removed the stale label Apr 5, 2024

removed the bug that occured during selecting date on mobile

2a13c8c

masterboy376 marked this pull request as ready for review April 5, 2024 14:57

masterboy376 added 2 commits May 29, 2024 19:54

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

7ff9152

…e-filter

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

c56ad66

…e-filter

nikitaevg requested changes May 30, 2024

View reviewed changes

oppiabot bot unassigned nikitaevg May 30, 2024

oppiabot bot assigned masterboy376 May 30, 2024

addressed Niks' comments

55db3b1

masterboy376 requested a review from nikitaevg May 30, 2024 10:37

oppiabot bot assigned nikitaevg and unassigned masterboy376 May 30, 2024

nikitaevg requested changes May 31, 2024

View reviewed changes

oppiabot bot unassigned nikitaevg May 31, 2024

oppiabot bot assigned masterboy376 May 31, 2024

masterboy376 added 2 commits May 31, 2024 14:49

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

69097f2

…e-filter

addressed Nik's comments

8491bcf

masterboy376 requested a review from nikitaevg May 31, 2024 09:45

oppiabot bot assigned nikitaevg and unassigned masterboy376 May 31, 2024

Merge remote-tracking branch 'upstream/develop' into adding-time-rang…

68acd6b

…e-filter

nikitaevg approved these changes Jun 2, 2024

View reviewed changes

oppiabot bot unassigned nikitaevg Jun 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #19570: Adding time range filter #19979

Fix part of #19570: Adding time range filter #19979

masterboy376 commented Mar 17, 2024 •

edited

oppiabot bot commented Mar 17, 2024

oppiabot bot commented Apr 5, 2024

oppiabot bot commented May 28, 2024

nikitaevg May 30, 2024

masterboy376 May 30, 2024

nikitaevg May 30, 2024

nikitaevg May 30, 2024

masterboy376 May 30, 2024

nikitaevg May 30, 2024

masterboy376 May 30, 2024

oppiabot bot commented May 30, 2024

oppiabot bot commented May 30, 2024

masterboy376 commented May 30, 2024

oppiabot bot commented May 30, 2024

nikitaevg left a comment

nikitaevg May 31, 2024

masterboy376 May 31, 2024

nikitaevg May 31, 2024

masterboy376 May 31, 2024

nikitaevg May 31, 2024

masterboy376 May 31, 2024

nikitaevg May 31, 2024

masterboy376 May 31, 2024

nikitaevg May 31, 2024

masterboy376 May 31, 2024

nikitaevg May 31, 2024

masterboy376 May 31, 2024

oppiabot bot commented May 31, 2024

oppiabot bot commented May 31, 2024

masterboy376 commented May 31, 2024

oppiabot bot commented May 31, 2024

oppiabot bot commented Jun 2, 2024

		<b *ngIf="activeTab === TAB_NAME_TRANSLATION_SUBMITTER \|\| activeTab === TAB_NAME_QUESTION_SUBMITTER" class="filter-label">Filter by Last Contributed after:</b>
		<b *ngIf="activeTab === TAB_NAME_TRANSLATION_REVIEWER \|\| activeTab === TAB_NAME_QUESTION_REVIEWER" class="filter-label">Filter by Last Reviewed after:</b>

		this.activeTab === this.TAB_NAME_QUESTION_REVIEWER \|\|
		this.activeTab === this.TAB_NAME_QUESTION_REVIEWER

Fix part of #19570: Adding time range filter #19979

Are you sure you want to change the base?

Fix part of #19570: Adding time range filter #19979

Conversation

masterboy376 commented Mar 17, 2024 • edited

Overview

Essential Checklist

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

oppiabot bot commented Mar 17, 2024

oppiabot bot commented Apr 5, 2024

oppiabot bot commented May 28, 2024

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

oppiabot bot commented May 30, 2024

oppiabot bot commented May 30, 2024

masterboy376 commented May 30, 2024

oppiabot bot commented May 30, 2024

nikitaevg 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

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

oppiabot bot commented May 31, 2024

oppiabot bot commented May 31, 2024

masterboy376 commented May 31, 2024

oppiabot bot commented May 31, 2024

oppiabot bot commented Jun 2, 2024

masterboy376 commented Mar 17, 2024 •

edited