Skip to content

Commit

Permalink
Fixed issues with tests for PR oppia#19264
Browse files Browse the repository at this point in the history
Created tests for PR oppia#19264 for the frontend and backend. generate_contributor_certificate_data in suggestion_services.py can now return None, so some function's return values were changed to Optional[ContributorCertificateInfoDict]. Tests were added on the frontend, and tests on the backend had to be changed to "assert response is not None" rather than "self.assertIsNotNone(response)" in order to satisfy the type checker. Notably, "None" also had to be added to the union type parameter in the render_json function in controllers/base.py for this reason as well.
  • Loading branch information
mgporter committed May 6, 2024
1 parent b466f4b commit 641a457
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 32 deletions.
2 changes: 1 addition & 1 deletion core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ def head(self, *args: Any, **kwargs: Any) -> None:
# Here we use type Any because the argument 'values' can accept various
# kinds of dictionaries that needs to be sent as a JSON response.
def render_json(
self, values: Union[str, Sequence[Mapping[str, Any]], Mapping[str, Any]]
self, values: Union[str, Sequence[Mapping[str, Any]], Mapping[str, Any], None]
) -> None:
"""Prepares JSON response to be sent to the client.
Expand Down
20 changes: 10 additions & 10 deletions core/domain/suggestion_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -3872,7 +3872,7 @@ def generate_contributor_certificate_data(
language_code: Optional[str],
from_date: datetime.datetime,
to_date: datetime.datetime
) -> suggestion_registry.ContributorCertificateInfoDict | None:
) -> Optional[suggestion_registry.ContributorCertificateInfoDict]:
"""Returns data to generate the certificate.
Args:
Expand All @@ -3887,7 +3887,8 @@ def generate_contributor_certificate_data(
contributions were created.
Returns:
ContributorCertificateInfoDict. Data to generate the certificate.
ContributorCertificateInfoDict (data to generate the certificate), or
None if no contributions are found.
Raises:
Exception. The suggestion type is invalid.
Expand Down Expand Up @@ -3919,7 +3920,7 @@ def _generate_translation_contributor_certificate_data(
from_date: datetime.datetime,
to_date: datetime.datetime,
user_id: str
) -> suggestion_registry.ContributorCertificateInfo | None:
) -> Optional[suggestion_registry.ContributorCertificateInfo]:
"""Returns data to generate translation submitter certificate.
Args:
Expand All @@ -3932,8 +3933,8 @@ def _generate_translation_contributor_certificate_data(
user_id: str. The user ID of the contributor.
Returns:
ContributorCertificateInfo. Data to generate translation submitter
certificate.
ContributorCertificateInfo (data to generate translation submitter
certificate), or None if no contributions are found.
Raises:
Exception. The language is invalid.
Expand Down Expand Up @@ -4013,7 +4014,7 @@ def _generate_question_contributor_certificate_data(
from_date: datetime.datetime,
to_date: datetime.datetime,
user_id: str
) -> suggestion_registry.ContributorCertificateInfo:
) -> Optional[suggestion_registry.ContributorCertificateInfo]:
"""Returns data to generate question submitter certificate.
Args:
Expand All @@ -4024,8 +4025,8 @@ def _generate_question_contributor_certificate_data(
user_id: str. The user ID of the contributor.
Returns:
ContributorCertificateInfo. Data to generate question submitter
certificate.
ContributorCertificateInfo (data to generate question submitter
certificate), or None if no contributions are found.
Raises:
Exception. The suggestion type given to generate the certificate is
Expand Down Expand Up @@ -4065,8 +4066,7 @@ def _generate_question_contributor_certificate_data(
hours_contributed = round(minutes_contributed / 60, 2)

if minutes_contributed == 0:
raise Exception(
'There are no contributions for the given time range.')
return None

return suggestion_registry.ContributorCertificateInfo(
from_date.strftime('%d %b %Y'), to_date.strftime('%d %b %Y'),
Expand Down
28 changes: 12 additions & 16 deletions core/domain/suggestion_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -7343,7 +7343,7 @@ def test_create_translation_contributor_certificate(self) -> None:
self.to_date,
)

self.assertIsNotNone(response)
assert response is not None
self.assertEqual(
response['contribution_hours'],
self._calculate_translation_contribution_hours(3)
Expand Down Expand Up @@ -7372,7 +7372,7 @@ def test_create_translation_contributor_certificate_for_rule_translation(
self.to_date,
)

self.assertIsNotNone(response)
assert response is not None
self.assertEqual(
response['contribution_hours'],
self._calculate_translation_contribution_hours(4)
Expand Down Expand Up @@ -7409,7 +7409,7 @@ def test_create_translation_contributor_certificate_for_english(
self.to_date,
)

self.assertIsNotNone(response)
assert response is not None
self.assertEqual(
response['contribution_hours'],
self._calculate_translation_contribution_hours(3)
Expand Down Expand Up @@ -7464,7 +7464,7 @@ def test_create_question_contributor_certificate(self) -> None:
self.to_date,
)

self.assertIsNotNone(response)
assert response is not None
self.assertEqual(
response['contribution_hours'],
self._calculate_question_contribution_hours(False)
Expand Down Expand Up @@ -7521,7 +7521,7 @@ def test_create_question_contributor_certificate_with_image_content(
self.to_date,
)

self.assertIsNotNone(response)
assert response is not None
self.assertEqual(
response['contribution_hours'],
self._calculate_question_contribution_hours(True)
Expand All @@ -7530,32 +7530,28 @@ def test_create_question_contributor_certificate_with_image_content(
def test_create_contributor_certificate_raises_exception_for_no_suggestions(
self
) -> None:
with self.assertRaisesRegex(
Exception,
'There are no contributions for the given time range.'
):
suggestion_services.generate_contributor_certificate_data(
data = suggestion_services.generate_contributor_certificate_data(
self.username,
feconf.SUGGESTION_TYPE_TRANSLATE_CONTENT,
'hi',
self.from_date,
self.to_date,
)
)

self.assertIsNone(data)

def test_create_certificate_raises_exception_for_no_question_suggestions(
self
) -> None:
with self.assertRaisesRegex(
Exception,
'There are no contributions for the given time range.'
):
suggestion_services.generate_contributor_certificate_data(
data = suggestion_services.generate_contributor_certificate_data(
self.username,
feconf.SUGGESTION_TYPE_ADD_QUESTION,
None,
self.from_date,
self.to_date,
)

self.assertIsNone(data)

def test_create_contributor_certificate_raises_exception_for_wrong_language(
self
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,25 @@ describe('Contributor Certificate Download Modal Component', () => {
);
});

it('should show error for no contributions found', fakeAsync(() => {
component.fromDate = '2020/01/01';
component.toDate = '2020/01/31';
spyOn(
contributionAndReviewService,
'downloadContributorCertificateAsync'
).and.returnValue(Promise.resolve(null));
spyOn(alertsService, 'addInfoMessage').and.stub();

component.downloadCertificate();

flushMicrotasks();

expect(component.errorsFound).toBeTrue();
expect(component.errorMessage).toEqual(
'There are no contributions for the given date range.'
);
}));

it('should show error for invalid date ranges', () => {
const today = new Date();
let tomorrow = new Date();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,13 @@ export class CertificateDownloadModalComponent {
this.fromDate,
this.toDate
)
.then((response: ContributorCertificateResponse) => {
.then((response: ContributorCertificateResponse | null) => {
if (response) {
this.createCertificate(response);
} else {
this.errorsFound = true;
this.errorMessage = 'There are no contributions for the given date range.';
this.errorMessage =
'There are no contributions for the given date range.';
}
this.certificateDownloading = false;
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export class ContributionAndReviewBackendApiService {
language: string | null,
fromDate: string,
toDate: string
): Promise<ContributorCertificateResponse> {
): Promise<ContributorCertificateResponse | null> {
const url = this.urlInterpolationService.interpolateUrl(
this.CONTRIBUTOR_CERTIFICATE_HANDLER_URL,
{
Expand All @@ -289,7 +289,7 @@ export class ContributionAndReviewBackendApiService {
params.language = language;
}
return this.http
.get<ContributorCertificateResponse>(url, {params} as Object)
.get<ContributorCertificateResponse | null>(url, {params} as Object)
.toPromise();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ export class ContributionAndReviewService {
languageCode: string | null,
fromDate: string,
toDate: string
): Promise<ContributorCertificateResponse> {
): Promise<ContributorCertificateResponse | null> {
return this.contributionAndReviewBackendApiService.downloadContributorCertificateAsync(
username,
suggestionType,
Expand Down

0 comments on commit 641a457

Please sign in to comment.