Moodle
  1. Moodle
  2. MDL-14591 META: Develop new Portfolio API
  3. MDL-16520

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

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.10
    • Component/s: Portfolio API
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      15445

      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

        Hide
        Martin Dougiamas added a comment -

        Added a screenshot

        Show
        Martin Dougiamas added a comment - Added a screenshot
        Hide
        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
        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
        Penny Leach added a comment -

        ready for QA.

        Show
        Penny Leach added a comment - ready for QA.
        Hide
        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
        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
        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
        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
        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)
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this issue.

        We have detected that this issue has been inactive for over a year. It was reported as affecting versions that are no longer supported.

        If you believe that this issue is still relevant to current versions (2.5 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

        Michael d.

        TW9vZGxlDQo=

        Show
        Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year. It was reported as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.5 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d. TW9vZGxlDQo=
        Hide
        Michael de Raadt added a comment -

        I'm closing this issue as it has been inactive for over a year has been recorded as affecting versions that are no longer supported.

        This is being done as part of a bulk annual clean-up of issues.

        If you still believe this is an issue in supported versions, please create a new issue.

        Show
        Michael de Raadt added a comment - I'm closing this issue as it has been inactive for over a year has been recorded as affecting versions that are no longer supported. This is being done as part of a bulk annual clean-up of issues. If you still believe this is an issue in supported versions, please create a new issue.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: