Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

There are no contributions for the given time range #19264

Open
kevintab95 opened this issue Dec 6, 2023 · 6 comments · May be fixed by #20261
Open

There are no contributions for the given time range #19264

kevintab95 opened this issue Dec 6, 2023 · 6 comments · May be fixed by #20261
Assignees
@mgporter
Labels
good first issue Impact: High Blocks or significantly slows down a core workflow. server errors Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@kevintab95
Copy link
Member

This error occurred recently in production:

Exception raised at https://www.oppia.org/contributorcertificate/<user-name>/translate_content?from_date=2021-09-05&to_date=2023-01-05&language=hi: There are no contributions for the given time range.
Traceback (most recent call last):
  File "/layers/google.python.pip/pip/lib/python3.8/site-packages/webapp2.py", line 604, in dispatch
    return method(*args, **kwargs)
  File "/workspace/core/controllers/acl_decorators.py", line 4788, in test_can_fetch_all_contributor_dashboard_stats
    return handler(self, username, **kwargs)
  File "/workspace/core/controllers/contributor_dashboard.py", line 1062, in get
    response = suggestion_services.generate_contributor_certificate_data(
  File "/workspace/core/domain/suggestion_services.py", line 3870, in generate_contributor_certificate_data
    data = _generate_translation_contributor_certificate_data(
  File "/workspace/core/domain/suggestion_services.py", line 3958, in _generate_translation_contributor_certificate_data
    raise Exception(
Exception: There are no contributions for the given time range.

Where did the error occur? Add the page the error occurred on.

Exception raised at https://www.oppia.org/contributorcertificate/<user-name>/translate_content?from_date=2021-09-05&to_date=2023-01-05&language=hi

Frequency of occurrence Add details about how many times the error occurred within a given time period (e.g. the last week, the last 30 days, etc.). This helps issue triagers establish severity.
4 times in a day.

General instructions for contributors
In general, the procedure for fixing server errors should be the following:

  • Analyze the code in the file where the error occurred and come up with a hypothesis for the reason.
  • Based on your hypothesis, determine a list of steps that reliably reproduce the issue (or confirm any repro instructions that have been provided). For example, if your hypothesis is that the issue arises due to a delay in a response from the backend server, try to change the code so that the backend server always has a delay, and see if the error then shows up 100% of the time on your local machine.
  • Explain your proposed fix, the logic behind it, and any other findings/context you have on this thread. You can also link to a debugging doc if you prefer.
  • Get your approach validated by an Oppia team member.
  • Make a PR that fixes the issue.
@chris7716 chris7716 added Impact: High Blocks or significantly slows down a core workflow. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Dec 11, 2023
@Ash-2k3 Ash-2k3 mentioned this issue Apr 3, 2024
Closed
@seanlip
Copy link
Member

seanlip commented Apr 3, 2024

Analysis from #19388 by @chris7716: Currently 'There are no contributions for the given time range.' is thrown as an internal server error. Modifying the function's signature to return ContributorCertificateInfo|None and have the caller handle the None case correctly would be the better solution.

@mgporter
Copy link

mgporter commented May 3, 2024

Hello! This is my first time contributing to Oppia. All of the documentation is definitely appreciated!

As @chris7716 mentioned, this error is thrown by the server when no contributions have been found. Suggestion_services.py sums the total word count of all translation contributions found, and if that word count is still zero, then the exception is raised.

Taking @chris7716's advice, I would change this check of the word count to instead return None, and then also change the calling method in the suggestion_services.py (generate_contributor_certificate_data) to return None as well in this case. Then contributor_dashboard.py can still do self.render_json(response), and if response was None, it will just be written as a null value that we can handle on the client side. Finally, in the client we handle it in certificate-download-modal.components.ts like this:

Screenshot 2024-05-03 000600

That should do it. By the way, at first, I wasn't able to view the Accomplishments tab in the Contributor Dashboard. I had to manually set accomplishmentsTabIsEnabled to true in order to test this (which obviously would not be part of this PR). Is there another way to enable the Accomplishments tab from within UI, or has this been disabled?

@seanlip
Copy link
Member

seanlip commented May 3, 2024 •
edited

@mgporter This looks like a good approach, thanks! I'm assigning you, feel free to make a PR :) (When can you make it, btw?)

Re the feature flag, sign in as an admin locally, then go to /admin > Roles and give yourself the Release Coordinator role. Then go to /release-coordinator to turn it on. (That said, the accomplishments tab is here to stay, so if you like I think it is fine to remove the feature flag altogether and set it to permanently display, but maybe that should be a separate PR.)

Hope that helps, and feel free to ask if you have any further questions -- thanks!

@mgporter
Copy link

mgporter commented May 3, 2024 •
edited

Thanks for the tip about the feature flag. I can make the PR within the next day most likely (I won't touch the feature flag right now though).

Working on this, I discovered a few other issues with the certificate-download-modal component. Would you mind if I made another PR and worked on these issues?

  1. The end date is exclusive, meaning that a contribution done today will only show up if we select tomorrow as the end date. I think this might be a bit unintuitive and would be better if the end date was inclusive.
  2. Since we would never want to accept dates in the future, we can just add a "max" value in the html for the datepicker and set it with today's date, rather than giving the user a warning when they select a future date.
  3. I feel like the most common use case of this modal would be to get all contributions. We could add radio buttons for two options: 1.) either select all contributions (would be default) or 2.) select a specific date range (which is what is there now).

@seanlip
Copy link
Member

seanlip commented May 3, 2024

@mgporter Thanks! Definitely open to these improvements; they sound good. Could you please file an issue for them, so that we can assign you to it?

Maybe file (1)+(2) as one issue and (3) as a separate one -- I'm not sure about what the UI for (3) would be, so let's validate that before assigning work on it.

@mgporter mgporter linked a pull request May 4, 2024 that will close this issue
Open
5 tasks
@mgporter
Copy link

mgporter commented May 4, 2024

Ok, just created the PR. I think I covered everything in the guidelines.

Thanks for the feedback- I'll make sure to do that!

mgporter added a commit to mgporter/oppia that referenced this issue May 6, 2024
@mgporter
Fixed issues with tests for PR oppia#19264
641a457 
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.
mgporter added a commit to mgporter/oppia that referenced this issue May 11, 2024
@mgporter
Addressing review comments for oppia#19264
67a0240 
Added CertificateDataResponse datatype to hold the certificate data or None. Added documentation for python asserts in tests. Some variable names, docstrings, and indentation were also updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mgporter mgporter

Labels
good first issue Impact: High Blocks or significantly slows down a core workflow. server errors Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Contributor Experience Team
Status: Todo
Milestone
No milestone
4 participants
@seanlip @kevintab95 @mgporter @chris7716