Inline problem response report status #19986

xitij2000 · 2019-03-13T08:51:27Z

Adds an inline status line while generating reports that reports progress. A continuation of https://github.com/edx/edx-platform/pull/18244

JIRA tickets: OSPR-3198

Discussions: https://github.com/edx/edx-platform/pull/18244

Screenshots:

Previous version

Sandbox URL:
LMS: https://pr19986.sandbox.opencraft.hosting/
Studio: https://studio.pr19986.sandbox.opencraft.hosting/

Merge deadline: None

Testing instructions:
TBD

Reviewers

edX reviewer[s] TBD

Settings

EDXAPP_FEATURES:
  ENABLE_COMBINED_LOGIN_REGISTRATION: true

openedx-webhooks · 2019-03-13T08:51:40Z

Thanks for the pull request, @xitij2000! I've created OSPR-3198 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

supporting documentation
edx-code email threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

natabene · 2019-03-13T15:03:42Z

@xitij2000 Thank you for your contribution. Please let me know once it is ready to be looked at.

xitij2000 · 2019-03-13T22:42:18Z

@marcotuts I've attached a gif capture of the intended interface in the ticket. If this seems OK, I will polish it up a bit more.

(The sandbox is also available for testing)

xitij2000 · 2019-03-13T22:47:16Z

@natabene I'd love a rough look at this point to see if the proposed UI style is OK. If so I will go ahead and complete the implementation.

natabene · 2019-03-14T17:04:33Z

@marcotuts Can you give this an early look?

marcotuts · 2019-03-17T23:50:48Z

This looks great! I have a few minor visual / UI tweaks and suggestions but it might be easier to draw out / visualize. By chance will you be at the conference in a few weeks? If not I'll reply back with a visual or summary of the minor suggestions. This is ok to move to engineering review next @natabene. This page needs a lot of overall work to simplify, but this is an incremental cleanup toward that direction.

xitij2000 · 2019-03-18T08:04:13Z

@marcotuts Yes, I will be at the conference.

I just wanted to show the general UX I was going for before finishing off details like padding etc. For one I'm not finding the progress text very useful, so I intended to remove that unless there is an error. Glad to know this is headed in the right direction!

xitij2000 · 2019-04-11T07:22:26Z

jenkins run js

marcotuts · 2019-04-12T18:22:29Z

At the final step, it isn't entirely clear that you select the lowest level item by clicking the blue link. All the way up to this point you are able to block on a block / button.

Separately, I will connect with the team to get feedback on the component itself, but this seems to be a valuable step in the right direction for making problem selection easier.

xitij2000 · 2019-04-13T09:05:54Z

@marcotuts That selection mechanism was added quite a while back (https://github.com/edx/edx-platform/pull/17824) in this PR I'm just adding the live status and link below this UI.

That said, I'd be open to making improvements to this UI in a separate PR (or this one if it's a small enough change).

I also wonder how useful the text field with the block key is, perhaps it can be de-emphasised.

natabene · 2019-04-16T14:41:24Z

@xitij2000 Can you let me know once all tests are green and this is ready for our review?

xitij2000 · 2019-04-16T14:45:51Z

@natabene I'm mainly waiting for the visual UI tweaks and suggestions for the inline status bit of the UI before I finalise this work.

xitij2000 · 2019-05-06T11:45:59Z

@marcotuts Would love some feedback on this so I can polish and finalise this.

xitij2000 · 2020-08-11T06:25:32Z

jenkins run python-3.8/python

bradenmacdonald · 2020-08-11T21:22:07Z

webpack.common.config.js

This may also not be needed anymore?

bradenmacdonald

Was just going to approve but noticed some potential a11y issues when going through my checklist. Sorry for not spotting that earlier. Please consider them, squash in a fix, and get the tests green then I can approve + merge.

lms/djangoapps/instructor/static/instructor/ProblemBrowser/components/Main/Main.jsx

...ngoapps/instructor/static/instructor/ProblemBrowser/components/ReportStatus/ReportStatus.jsx

Adds an in-line status display for problem report generation progress. Co-authored-by: Adam Kovari <adam@opencraft.com> Co-authored-by: Eugeny Kolpakov <eugeny.kolpakov@gmail.com> Co-authored-by: Kshitij Sobti <kshitij@sobti.in> Co-authored-by: Braden MacDonald <braden@opencraft.com>

edx-status-bot · 2020-08-12T14:04:33Z

Your PR has finished running tests. There were no failures.

xitij2000 · 2020-08-12T20:33:34Z

@bradenmacdonald This is ready for review. I did not make any changes to the text input since it didn't make sense for a disabled input, but if you feel there are changes needed there, do tell.

bradenmacdonald

👍

I tested this: on the sandbox
I read through the code
I checked for accessibility issues
Includes documentation: n/a

openedx-webhooks · 2020-08-13T16:51:59Z

@xitij2000 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

edx-pipeline-bot · 2020-08-13T18:32:00Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

xitij2000 · 2020-08-13T21:44:50Z

Thanks!

edx-pipeline-bot · 2020-08-13T22:26:26Z

EdX Release Notice: This PR has been deployed to the production environment.

DawoudSheraz · 2020-08-28T07:35:38Z

@xitij2000 Hi. Since the merge of this PR, we have been experiencing the issues in problem response CSV generation. There have been about 200 instances of the following error ever since this went live on production:

Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python3.5/contextlib.py", line 30, in inner
    return func(*args, **kwds)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.5/site-packages/newrelic/hooks/framework_django.py", line 554, in wrapper
    return wrapped(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/views.py", line 80, in instructor_task_status
    output = _get_instructor_task_status(task_id)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/views.py", line 27, in _get_instructor_task_status
    instructor_task = get_updated_instructor_task(task_id)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/api_helper.py", line 292, in get_updated_instructor_task
    _update_instructor_task(instructor_task, result)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/api_helper.py", line 219, in _update_instructor_task
    task_output = InstructorTask.create_output_for_success(returned_result)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/instructor_task/models.py", line 151, in create_output_for_success
    raise ValueError(u"Length of task output is too long: {0}".format(json_output))
ValueError: Length of task output is too long: {"total": 1, "failed": 0, "skipped": -34, "step": "CSV uploaded", "succeeded": 35, "preassigned": 0, "action_name": "generated", "attempted": 35, "duration_ms": 901}

Can you please take a look and either fix/revert the PR? The feature is broken on the platform and is becoming a blocker. Let me know if you think otherwise. Thanks

xitij2000 · 2020-08-28T11:48:21Z

@DawoudSheraz I'm not sure how this PR could have affected this, and am a little confused by the error.

It seems to be thrown here: https://github.com/edx/edx-platform/blob/75495fd00314a42b7f27c81768f55ac7b18e5977/lms/djangoapps/instructor_task/models.py#L150-L151
It seems to be thrown if the task output is too long, but the output printed above is way smaller than 1024 characters.

This PR is mostly a UI change, and while it does add the report path to the tasks metadata, I don't see in the task output above. Would you have any context on what might be going on? I will look into fixing it accordingly.

DawoudSheraz · 2020-08-28T13:08:35Z

@DawoudSheraz I'm not sure how this PR could have affected this, and am a little confused by the error.

It seems to be thrown here:

https://github.com/edx/edx-platform/blob/75495fd00314a42b7f27c81768f55ac7b18e5977/lms/djangoapps/instructor_task/models.py#L150-L151

It seems to be thrown if the task output is too long, but the output printed above is way smaller than 1024 characters.
This PR is mostly a UI change, and while it does add the report path to the tasks metadata, I don't see in the task output above. Would you have any context on what might be going on? I will look into fixing it accordingly.

@xitij2000 The task status on success now also includes key report_path, whose value is S3 temporary link. I had removed it from the traceback that I shared earlier. The link is very large and is resulting in server error when getting the resulting dict.

xitij2000 · 2020-08-28T13:22:06Z

@DawoudSheraz OK, that makes sense then. I will investigate an alternative approach to this to avoid needing that in the task output.

DawoudSheraz · 2020-08-28T13:23:28Z

@xitij2000 Thanks.

xitij2000 · 2020-08-28T14:52:34Z

@DawoudSheraz I have a PR that fixes this: https://github.com/edx/edx-platform/pull/24867
I've removed the report path from the task output and am instead fetching it from a different API in the frontend.

DawoudSheraz · 2020-08-28T15:54:55Z

@xitij2000 Thanks for the quick turnaround.

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 13, 2019

xitij2000 changed the title ~~Kshitij/report status~~ [WIP] Inline problem response report status Mar 13, 2019

openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs triage labels Mar 13, 2019

openedx-webhooks added product review PR requires product review before merging and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 14, 2019

openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed product review PR requires product review before merging labels Mar 18, 2019

xitij2000 force-pushed the kshitij/report-status branch 4 times, most recently from a092b66 to c1932c0 Compare April 6, 2019 00:28

xitij2000 force-pushed the kshitij/report-status branch 2 times, most recently from 1d20ca9 to 9ad66a0 Compare April 11, 2019 06:40

openedx-webhooks added product review PR requires product review before merging and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels May 6, 2019

xitij2000 force-pushed the kshitij/report-status branch from 597c45f to 0efbeab Compare August 11, 2020 04:56

bradenmacdonald reviewed Aug 11, 2020

View reviewed changes

webpack.common.config.js Outdated

Copy link

Contributor

bradenmacdonald Aug 11, 2020

Choose a reason for hiding this comment

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

This may also not be needed anymore?

bradenmacdonald reviewed Aug 11, 2020

View reviewed changes

lms/djangoapps/instructor/static/instructor/ProblemBrowser/components/Main/Main.jsx Outdated Show resolved Hide resolved

...ngoapps/instructor/static/instructor/ProblemBrowser/components/ReportStatus/ReportStatus.jsx Outdated Show resolved Hide resolved

natabene added open edx community review and removed needs triage labels Aug 12, 2020

xitij2000 force-pushed the kshitij/report-status branch from 0efbeab to b8167b7 Compare August 12, 2020 06:46

xitij2000 force-pushed the kshitij/report-status branch from b8167b7 to 559ff54 Compare August 12, 2020 13:35

bradenmacdonald approved these changes Aug 12, 2020

View reviewed changes

ormsbee merged commit 5d33501 into openedx:master Aug 13, 2020

openedx-webhooks added needs triage merged and removed open edx community review labels Aug 13, 2020

xitij2000 deleted the kshitij/report-status branch August 13, 2020 21:44

xitij2000 mentioned this pull request Sep 8, 2020

[BB-2967] Remove report path from task output data #24867

Merged

2 tasks

Inline problem response report status #19986

Inline problem response report status #19986

Uh oh!

Conversation

xitij2000 commented Mar 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented Mar 13, 2019

Uh oh!

natabene commented Mar 13, 2019

Uh oh!

xitij2000 commented Mar 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xitij2000 commented Mar 13, 2019

Uh oh!

natabene commented Mar 14, 2019

Uh oh!

marcotuts commented Mar 17, 2019

Uh oh!

xitij2000 commented Mar 18, 2019

Uh oh!

xitij2000 commented Apr 11, 2019

Uh oh!

marcotuts commented Apr 12, 2019

Uh oh!

xitij2000 commented Apr 13, 2019

Uh oh!

natabene commented Apr 16, 2019

Uh oh!

xitij2000 commented Apr 16, 2019

Uh oh!

xitij2000 commented May 6, 2019

Uh oh!

xitij2000 commented Aug 11, 2020

Uh oh!

bradenmacdonald Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

edx-status-bot commented Aug 12, 2020

Uh oh!

xitij2000 commented Aug 12, 2020

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

openedx-webhooks commented Aug 13, 2020

Uh oh!

edx-pipeline-bot commented Aug 13, 2020

Uh oh!

xitij2000 commented Aug 13, 2020

Uh oh!

edx-pipeline-bot commented Aug 13, 2020

Uh oh!

DawoudSheraz commented Aug 28, 2020

Uh oh!

xitij2000 commented Aug 28, 2020

Uh oh!

DawoudSheraz commented Aug 28, 2020

Uh oh!

xitij2000 commented Aug 28, 2020

Uh oh!

DawoudSheraz commented Aug 28, 2020

Uh oh!

xitij2000 commented Aug 28, 2020

Uh oh!

DawoudSheraz commented Aug 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

xitij2000 commented Mar 13, 2019 •

edited

Loading

xitij2000 commented Mar 13, 2019 •

edited

Loading