Embed questions from system context

[andrewhancox]

https://github.com/moodleou/moodle-filter_embedquestion/blob/c549a4255fc05b815b336f8f266a2e28987fb1ed/internaldoc/functionality.txt#L30

Is there a specific reason for this limitation? I've been asked to create a patch to allow a client to embed questions from the system question bank but am a bit wary as you seem to have very intentionally not allowed it...

[timhunt]

The goal with these plugins was to build something that worked for our users in a way that was not too complicated (since that leads to uimple UI, and simple code => few bugs).

Bear in mind that the embed code is based on idnumbers of the question category and question. Question category idnumber is only guaranteed to be unique within one context.

So, not a fundamental assumption, but if you want the changes integrated here (which hopefully you do) the complexity must not increase too much. Also, remember that you need to work any changes through to the related atto and report plugins.

I could try to comment on a summary of how you propose to implement your change, if you think that would be helpful.

[andrewhancox]

Thanks for the speedy response and the heads up about uniqueness of id numbers. I’ll take a crack at the change in the next few days.

Sent from my iPhone

On 30 Apr 2020, at 18:41, Tim Hunt notifications@github.com wrote:

 The goal with these plugins was to build something that worked for our users in a way that was not too complicated (since that leads to uimple UI, and simple code => few bugs).

Bear in mind that the embed code is based on idnumbers of the question category and question. Question category idnumber is only guaranteed to be unique within one context.

So, not a fundamental assumption, but if you want the changes integrated here (which hopefully you do) the complexity must not increase too much. Also, remember that you need to work any changes through to the related atto and report plugins.

I could try to comment on a summary of how you propose to implement your change, if you think that would be helpful.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[andrewhancox]

I’ve spent some time looking at this and there are two main threads to the changes needed:

1) Enhance the capabilities checks so that they are looking at question:useall and question:usemine in the course, category and system contexts and filtering categories/questions as appropriate. I’ve done this for one scenario already, I just need to refactor/centralise and apply the same restrictions elsewhere.

2) Come up with a workaround for the ID number uniqueness issue you mentioned. This is a bit sticky. The only obvious way I can see around this would be to add the course/coursecategory idnumber or ’SITE' to the embed code that gets generated, if there is none then assume it’s the course context to avoid issues with existing embeds. Does this sound sensible? I can’t see another way around it...

On 30 Apr 2020, at 18:46, Andrew Hancox andrewdchancox@googlemail.com wrote:

Thanks for the speedy response and the heads up about uniqueness of id numbers. I’ll take a crack at the change in the next few days.

Sent from my iPhone

On 30 Apr 2020, at 18:41, Tim Hunt notifications@github.com wrote:



The goal with these plugins was to build something that worked for our users in a way that was not too complicated (since that leads to uimple UI, and simple code => few bugs).

Bear in mind that the embed code is based on idnumbers of the question category and question. Question category idnumber is only guaranteed to be unique within one context.

So, not a fundamental assumption, but if you want the changes integrated here (which hopefully you do) the complexity must not increase too much. Also, remember that you need to work any changes through to the related atto and report plugins.

I could try to comment on a summary of how you propose to implement your change, if you think that would be helpful.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/moodleou/moodle-filter_embedquestion/issues/10#issuecomment-622001085, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJEYMRLH6F3VCWIACRIFHLRPGZ3PANCNFSM4MVZUEDQ.

[andrewhancox]

Actually, we could embed the context level enum value, as we know the course already and it can only be in one category/site.... Do you agree? That swerves the issue around idnumber not being mandatory on courses/categories and short names being liable to change.

The only scenario would where that could still cause a collision would be two question categories with the same idnumber existing in two course categories nested within each other... which would be the edgiest of edge cases.

[timhunt]

I don't see why it is an issue that short names can change. We will never embed questions from course C2 into course C1. If the question is coming from course context, it will only every be current course context (which would be front-page, if nothing else.)

But, yes, we need to extend the embed-id information a bit. As you say, add a $contextlevel field to class embed_id. If you make it a third argument to the constrcutor with default CONTEXT_COURSE, that keeps backwards compt. (We have some OU custom code which automated our publishing that it would be nice not to break :-))

We also need to extend the syntax for embed codes, which currently looks like:

{Q{catid/qid|---}Q} -- for course id

Since people will have these codes in existing content in their Moodle sites, then we cannot change that, and I don't think we need to. We just need to extend it. Perhpas

{Q{S/catid/qid|---}Q} -- for system context

Do you actually need course category context? That is tricker, because a course is only in some course categories. One option is to leave that until someone really needs it. Or, if you want, we can think about it now.

You also then need to think about the embed form (used in both filter/embedquestion/testhelper.php and atto plugin) - which needs to work niecly for both people with, and whitout, access to the system qusetion bank.

[timhunt]

Given https://docs.moodle.org/dev/Question_bank_improvements_for_Moodle_4.0 is coming, I don't think it makes sense to change how embed question system works to support the current sharing model. Once 4.0 is out, then we will have to work out a new approach. So, I will close this for now.