Hello. I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

If you have any information about this issue or a possible fix please post it here

Up for peer review. Note that this branch is based on MDL-50191

Fails against automated checks.

Checked MDL-28954 using repository: https://github.com/marinaglancy/moodle

• MOODLE_29_STABLE (2 errors / 2 warnings) [branch: wip-MDL-28954-m29 | CI Job]
  • phplint (0/0), php (1/2), js (0/0), css (0/0), phpdoc (0/0), commit (1/0), savepoint (0/0), thirdparty (0/0),

_{More information about this report}

Hi Marina, thanks for work on this.

1. I've noticed the you've added the same changes to the cohort/index.php here, is there any reason for that?
   I know this issue is blocked by the MDL-50191, but I think the patch should be rebased on top of those changes.
2. Another point is few calls like this:

   cohort_update_cohort((object)array('id' => $data->id, 'description' => $data->description, 'contextid' => $context->id));

   Not against declare and pass an array as parameter, I'm not a great fan of forcing it as (object) there. Up to you.

Rest looks good. Feel free to push for integration.

Thanks.

Thanks Simey, I change the (object)

TO INTEGRATOR: please note that the first commit in the branches is from another issue that is blocking this

Fails against automated checks.

Checked MDL-28954 using repository: https://github.com/marinaglancy/moodle

• MOODLE_28_STABLE (2 errors / 2 warnings) [branch: wip-MDL-28954-m28 | CI Job]
  • phplint (0/0), php (1/2), js (0/0), css (0/0), phpdoc (0/0), commit (1/0), savepoint (0/0), thirdparty (0/0),
• MOODLE_29_STABLE (2 errors / 2 warnings) [branch: wip-MDL-28954-m29 | CI Job]
  • phplint (0/0), php (1/2), js (0/0), css (0/0), phpdoc (0/0), commit (1/0), savepoint (0/0), thirdparty (0/0),
• master (2 errors / 2 warnings) [branch: wip-MDL-28954-master | CI Job]
  • phplint (0/0), php (1/2), js (0/0), css (0/0), phpdoc (0/0), commit (1/0), savepoint (0/0), thirdparty (0/0),

_{More information about this report}

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

This seems like an improvement rather than a bugfix to me? I don't imagine this was ever supported.

I agree with Dan, yesterday while looking to the linked issue I came here and thought the same. If, till now it has been impossible to add images to cohort descriptions I'd consider this a new feature/improvement.

Also, when I looked to code I had a thought about why the editor does show/allow to click the images/media button if the instance has been explicitly configured to avoid embed files. Maybe we should be dimming/hiding the button?

Ciao

I'll leave it up to integrator to decide whether it's a bug or improvement.

As for the images/media button - you are still able to insert an image/media/link specifying URL

Stopping review of this and converting to an improvement - we'll follow the procedure - integrate this next week and can have a backport request if it wants to go to stables.

Now that the on-sync period has ended, this issue is being un-held and will be processed normally as part of the integration queue. Thanks for your patience.

Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

Fails against automated checks.

Checked MDL-28954 using repository: https://github.com/marinaglancy/moodle

• MOODLE_28_STABLE (1 errors / 0 warnings) [branch: wip-MDL-28954-m28 | CI Job]
  • phplint (0/0), php (0/0), js (0/0), css (0/0), phpdoc (0/0), commit (1/0), savepoint (0/0), thirdparty (0/0),
• MOODLE_29_STABLE (0 errors / 0 warnings) [branch: wip-MDL-28954-m29 | CI Job]
• master (1 errors / 0 warnings) [branch: wip-MDL-28954-master | CI Job]
  • phplint (0/0), php (0/0), js (0/0), css (0/0), phpdoc (0/0), commit (1/0), savepoint (0/0), thirdparty (0/0),

_{More information about this report}

Code verified against automated checks.

Checked MDL-28954 using repository: https://github.com/marinaglancy/moodle

• master (0 errors / 0 warnings) [branch: wip-MDL-28954-master | CI Job]

_{More information about this report}

Integrated (master only), thanks!

Thanks for fixing this Marina,

Works as expected, images can be changed/added and retained when changing context. Also, user with cohort/view can only view images, rest get "filenotfound" error.

Passing...

Thanks for your contributions and hard work! This change is now available from the main moodle.git repository and will shortly be available on download.moodle.org.

“Anyone who has never made a mistake has never tried anything new.”
– Albert Einstein

MDLQA-7158 has been modified to ensure this change is tested.