|
problem with supporting multiple exports per session:
15:29 < penny> gah, I just remembered one reason that two exports it the same session are not supported The more I think about this the more I am convinced I was right in the first place to not support multiple exports per session. We cannot assume that portfolio plugins are going to support multiple exports per session, therefore I don't think Moodle should.
Just committed a patch for this.
I'm now storing the id of the export (matching portfolio_tempdata.id) in the session only. Petr if you can think of a clever work around for the problem I outlined above this is pretty easy to change later, but at the moment I'm stuck thinking we're limited to one per session. I do not agree, session is not suitable for passing of information from one page to another, especially if used from several different contexts. The problem that some plugins do not allow concurrent communication is not solved by sessions, what if somebody opens two different browsers and works with them at the same time? I think it should be better to find out what is going by looking into portfolio tables instead of session.
I really think you should use id from some table and pass it through all forms and build everything around this it. You might still use the session (or some serialised table field) to store the the auxiliary data, but the uniquie id is imho the key to make it all work. (ex. :$SESSION->prtfolioexportdata[$exportrequestid], I still do not like the serialisation of full blown objects either in events or session, I like arrays more there ) You said you want to store the unique id in session, I think you should instead use the unique id to "find" you information in session. I'm NOT storing data in session, just the portfolio request id, which is the id of the data.
The only thing I changed my mind about was not supporting multiple exports during the same session, for the reasons outlined above. The only thing that is now stored in the session is the portfolioexport id. Just to be 100% explicit - storing the id in the session rather than passing it around in forms or as a request parameter - because that is not possible to do with all plugins. (what you call 'lazy')
"because that is not possible to do with all plugins" - are you sure? As long as it is PHP or any other code stored on Moodle server it should be imho possible
The difference above in session use is: The 1/ does not allow you to work with two different pages at the same time; 2/ keeps separate state for different pages depending on the unique id, in case of 2/ you can also store the data in database Petr, I'm not sure you understand the problem correctly.
Some plugins take you away from moodle entirely and then redirect you back to a url that is preconfigured per installation and does not include any dynamic content (eg portfoliotransferid) - in that case, when you land back at portfolio/add.php there is no way to determine which portfolio transfer you are returning to. Unless you are only supporting one. Eg box.net redirects you to wwwroot/portfolio/add.php?returncontrol=1 or something - that is configured at box.net and is not able to be configured per redirect-request. Eg no way to include the id of the current transfer it relates to. hmm, they do not allow you to pass some extra information
 ok, some would support only one active transfer - does it mean we must limit all of them? Just started to discover box.net, if I understand it correctly box.net may be configured to send back more information, right? Am I reading the right docs section? I was reading: http://enabled.box.net/docs/rest#authentication
 which is how to authenticate users (which is the exact step that is causing this problem). Try going to enabled.box.net and setting up an account (read http://docs.moodle.org/en/admin/manageportfolio/boxnet for instructions) - I cannot see how to make this include dynamic data to the return url.
As far as: > ok, some would support only one active transfer - does it mean we must limit all of them? I honestly don't think that the use case for exporting multiple times at the same time is large enough to justify worrying about it this much. Perhaps we can elicit some response from Martin about this tomorrow. Everything has been converted to use files api, except for the following outstanding issues:
1. forum module (the rest of it doesn't seem to have been converted so I will wait) todo still:
last data module changes have been done. the ods/xls stuff isn't done but i'm making another bug for that.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sub-task
Resolved
Minor
 |
|
Currently this table is opt-in for callers that want to access a lot of data across requests, but now I will switch to storign everything in it rather than the session.
There needs also to be a cronjob to clean up records in this table from crashed requests (and associated files)