1. Oki, so it goes to "imslti", thanks (I'm sorry I saw in your commits already two name changes, lol, so this will be the 3rd). About the "External tool" name, I had in my internal notes that addressed too (it became lost apparently), and I think it's better to name it "IMS LTI" why not? It's exactly that.
2. So, if we consider this a brand-new module, then surely we don't need: i) any upgrade steps, everybody is going to install it from scratch. ii) any conversion from moodle 1.x backups (I saw one commit from Darko about that). Or do you need any of i) or ii) ?
3. Oki, if I get some time over the weekend I'll try to sort as much as possible of this and will put one commit on top of you branch (I think I've write perms there).
4. Well, the solution here is to have one unified library @ core (see C1), so any plugin can use it. I've created MDL-30149 about that. So surely, although we don't use namespaces... until C1 is solved (for Moodle 2.3), using them is the only solution to certainly avoid any conflict. Just if you can put here and there some:
// TODO: Switch to core oauthlib once implemented - MDL-30149
5. Hehe, no war at all, I've been using here documents since ages ago, specially for shell scripts, were concat operations are horrible. But in Moodle, simply, we don't use them. Yes, there are some uses, but please, ignore them. Specially the lang strings... that stuff goes to AMOS (our system for supporting translations and language packs generation, and I bet it won't "digest" them properly at all). So, for sure, the lang ones must be out, and also the SQLs (using double/single quotes as needed). About the HTML ones, I can live with them if they require variables substitution, but no excuse if they are pure HTML IMO. Once again, if I get some time... I'll try to help here (I love tedious and repetitive tasks).
2. Are there any specific recommendations for changes in this file? Well, I must confess that the bit calling my attention was:
$rawbody = file_get_contents("php://input");
that basically means it accepts any content. And the point was about to look carefully how we are validating/using/storing that information. So nothing found yet (I'm about to start 2nd, deeper, iteration). I'll keep you informed if I find anything annoying.
3. I think I was looking @ this some weeks ago and there were not proper solutions for the dynamic icon thing. All that stuff is controlled by our mod_info/cm_info core stuff and it will need to be able to accept "other" icons. But I'm not sure if that "other" should include remote urls or just pulled & locally stored icons. It is something to discuss and I'm 90% sure this won't be possible in Moodle 2.2. I'll try to introduce it @ HQ next week to see which are the feelings about it. Depending of that, perhaps we'll need to comment out some parts of the UI (hopefully re-introducing them ASAP).
4. Well, perhaps the problem is in the "IMS LTI 1.0 PHP basic provider" itself (the one I used for basic testing), behaving wrongly. I agree that json format itself should be ok/safe. But, if we can make it safer just by using another, simpler, format, or by encoding in another way.. then my +1 goes to make it (we don't know what some providers outa there are going to accept).
5. Yeah, that would be terrible welcome, I really was not able to get the point beyond that stuff. I guessed is was sort of pre-configured external tools available to all activities, but the "pending/rejected" thing, and the organizations... simply didn't fit in my (limited) brain.
1. See A4: MDL-30149
3. Oki, I think we can survive without those scroll bars (I sounded to me a lot to the fights I had to maintain some years ago in the IMS-CP resource type). So we can close this as "Done!" if nobody objects it.
4. Yes, what I mean is that I get four options in the "Launch container" menu: ("default", "embed", "embed without blocks" and "new window") and it's not clear at all the meaning of the "default" one (or from where that default comes/which is its current value). That leaded me to think it that something is wrong with it.
Finally, about the pending things, I'd recommend to, at this time, center our efforts in the "required" stuff. Where required means all we need to get this integrated, aka (A) and as most as possible of (B). Once being there, we'll have 3 more weeks for testing, fixing (B) and add new stuff (C/subtasks).
Of course, big thanks for your hard work! Let's push, push, push! And happy good trip back from JP!
I'm back to the 2nd review round (will try to fix trivial coding-rulezz while doing). Anything new found will be documented in the wiki and shared here.