Moodle

Failed portfolio exports result in an ugly error message that users should never see

Details

  • Type: Sub-task Sub-task
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0.8
  • Component/s: Portfolio API
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

1) Start an export from a forum post to Box.net
2) Change your mind when authenticating
3) Go back to the forum discussion
4) Observe the carnage (error message contains CONTROLS and also prevents forum display)

I think it should still show the forum discussion, and perhaps have a little alerting "!" icon or something next to the "Add to portfolio" link to show that it's in progress and some , clicking on it would go to a separate screen that tells you what is going on and lets you cancel/complete etc.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Martin Dougiamas added a comment -

Added a screenshot

Show
Martin Dougiamas added a comment - Added a screenshot
Hide
Permalink
Penny Leach added a comment -

i committed a fix to HEAD for this. currently there are 4 types of displays for the portfolio button and I could use a bit of help on the wording for some of them.

1. full form (includes dropdown of plugins) + button
2. dropdown of plugins + icon
3. icon by itself
4. text link

2 and 3 are fine, as I just put a ! on the existing portfolio icon (ignoring the fact that both of these will need to change probably)

1 and 4 share the same text, which is currently 'Add to portfolio - needs attention' which I'm not happy with.

I played with making it always be the icon by itself but in some places you really do want the button and the link.

At any rate, the code to handle this is done and the display is in lib/portfoliolib (already_exporting method on button class) and the wording is in language strings so very trivial to change later if someone has brilliant ideas about wording.

Martin I'm assigning you QA on this, please close the bug if you're happy with the reimplementation.

Show
Penny Leach added a comment - i committed a fix to HEAD for this. currently there are 4 types of displays for the portfolio button and I could use a bit of help on the wording for some of them. 1. full form (includes dropdown of plugins) + button 2. dropdown of plugins + icon 3. icon by itself 4. text link 2 and 3 are fine, as I just put a ! on the existing portfolio icon (ignoring the fact that both of these will need to change probably) 1 and 4 share the same text, which is currently 'Add to portfolio - needs attention' which I'm not happy with. I played with making it always be the icon by itself but in some places you really do want the button and the link. At any rate, the code to handle this is done and the display is in lib/portfoliolib (already_exporting method on button class) and the wording is in language strings so very trivial to change later if someone has brilliant ideas about wording. Martin I'm assigning you QA on this, please close the bug if you're happy with the reimplementation.
Hide
Permalink
Penny Leach added a comment -

Hmm, I guess I could still display the button or link as they are with a ! or the icon ... I'll keep playing with it.

Show
Penny Leach added a comment - Hmm, I guess I could still display the button or link as they are with a ! or the icon ... I'll keep playing with it.
Hide
Permalink
Penny Leach added a comment -

ready for QA.

Show
Penny Leach added a comment - ready for QA.
Hide
Permalink
Dan Poltawski added a comment -

Just a note that at the moment you can:

  • Run a forum portfolio export and give up in the middle
  • Hop along to a glossary
  • Run porfolio export and see the 'in progress' warning but also a big friendly 'Add to Portfolio' button like add_warning screenshot

This will then allow you to cancel or continue the existing portfolio export - however if I cancel it, it will return me to the forum rather than the glossary which I was just trying to export (and cancel all existing session).

Show
Dan Poltawski added a comment - Just a note that at the moment you can:
  • Run a forum portfolio export and give up in the middle
  • Hop along to a glossary
  • Run porfolio export and see the 'in progress' warning but also a big friendly 'Add to Portfolio' button like add_warning screenshot
This will then allow you to cancel or continue the existing portfolio export - however if I cancel it, it will return me to the forum rather than the glossary which I was just trying to export (and cancel all existing session).
Hide
Permalink
Penny Leach added a comment -



To clarify, I think the best solution is to take away the offer to add anything anywhere when there's an active session, and go back to having JUST the "!" link which takes you to the helpful page (although then it's still going to bounce you back to forum, not glossary) but at least it follows more logically.

Show
Penny Leach added a comment - To clarify, I think the best solution is to take away the offer to add anything anywhere when there's an active session, and go back to having JUST the "!" link which takes you to the helpful page (although then it's still going to bounce you back to forum, not glossary) but at least it follows more logically.
Hide
Permalink
Penny Leach added a comment -

code-wise there are two things here.

1. portfolio_button->print_already_exporting (handles the drawing of the ! stuff)

2. porttfolio_exporter->cancel_request

the second one is the problem. it doesn't know whether you've cancelled a portfolio export in-line (logically return to forum in dan's case), or if you've reawakened one from somewhere else (glossary in dan's case) and you should go back there.

the only thing I can really think of doing is make 1. add an extra parameter to 2. to bounce somewhere else ratehr than the return url of the original export.

(running out of time )

Show
Penny Leach added a comment - code-wise there are two things here. 1. portfolio_button->print_already_exporting (handles the drawing of the ! stuff) 2. porttfolio_exporter->cancel_request the second one is the problem. it doesn't know whether you've cancelled a portfolio export in-line (logically return to forum in dan's case), or if you've reawakened one from somewhere else (glossary in dan's case) and you should go back there. the only thing I can really think of doing is make 1. add an extra parameter to 2. to bounce somewhere else ratehr than the return url of the original export. (running out of time )
Hide
Permalink
Martin Dougiamas added a comment -

Following our longish talk just now, I think it would be a lot simpler if we just don't worry about showing all already.php at all. We should just destroy the incomplete export (with a little message that we did so) and plough on with the new export.

Exports are such small things (user-wise) all this complication in the interface doesn't seem worth it. Keep it simple.

Might have to have something polite happen just in case they go back to another open window and try to submit a form on the now-destroyed export session (such as a notice like "this export has timed out, start again").

Perhaps just comment out relevant code in case I'm completely wrong.

Thanks Penny! (hug)

Show
Martin Dougiamas added a comment - Following our longish talk just now, I think it would be a lot simpler if we just don't worry about showing all already.php at all. We should just destroy the incomplete export (with a little message that we did so) and plough on with the new export. Exports are such small things (user-wise) all this complication in the interface doesn't seem worth it. Keep it simple. Might have to have something polite happen just in case they go back to another open window and try to submit a form on the now-destroyed export session (such as a notice like "this export has timed out, start again"). Perhaps just comment out relevant code in case I'm completely wrong. Thanks Penny! (hug)
Hide
Permalink
Penny Leach added a comment -

I just made it so that when an exception happens the incomplete export is automatically killed when the 'export failed' screen is shown, which really helps (and I can't see why it wasn't like that already, but I put a comment about it in lib/portfolio/exceptions.php)

Show
Penny Leach added a comment - I just made it so that when an exception happens the incomplete export is automatically killed when the 'export failed' screen is shown, which really helps (and I can't see why it wasn't like that already, but I put a comment about it in lib/portfolio/exceptions.php)

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.