03 December 2015, 4:33

Not sure if this is a bug or a feature.

Issue:

Create an artefact (plan, journal, etc)

Create a page and put the artefact you just created on that page.

Go back to artefact editing (i.e. if you created a journal go to Content/journal and delete the journal)

Go back to page you created above and the block instance remains with a title [No Title]. If you click on settings for that block it tells you no artefacts exist. 

I could see where this might be a design call the idea being that if you delete an artefact you may want the block to act as a placeholder on the page where you replace it with another artefact of the same class (if there are any). Or you can easily delete the block in page edit mode and replace it with something else. 

The issue (if it is one) is happening in BlockInstance::bulk_delete_records on line 1453 where it sets configdata['artefactId'] to Null instead of deleting the record from block_instance. From the name of the method (bulk_delete_records) I would have guessed this would delete the record from block_instance where configdata['artefactid'] = deleted_artefact_id but the line 1453 in blocktype/lib.php does the following:

set_field('block_instance', 'configdata', serialize($blockdata->configdata), 'id', $blockid);

I stumbled across this when creating my own artefact plugin and thought I had a bug when deleting artefacts.

Just writing this out leads me to believe this is by design, but I thought I'd mention it in case I'm missing something.

23 December 2015, 12:53

Hello Gary,

I can't comment on the code, but leaving an empty block is by design. It helps creating template pages where you already provide the structure but not the actual content yet.

Cheers

Kristina

15 January 2016, 12:48

The function in question, "BlockInstance::bulk_delete_artefacts($records)", was written in 2009 before I started on the Mahara project, but I think the strategy of leaving the empty blocks behind is indeed on purpose.

It's kind of a compromise solution, I think. Obviously this isn't perfect, because it leaves that empty block in place, and sometimes you don't want that. On the other hand, removing the blocks wouldn't be an ideal solution either, because it can mess up the page layout. And having that empty block on a page may serve as a placeholder to remind you to put something new there.

I agree that the method is confusingly mis-named though. It isn't deleting artefacts at all. It's just removing them from block instances, so it should more accurately be called "bulk_remove_artefacts". I've whipped up a quick patch to refactor it a little for 16.04: https://bugs.launchpad.net/mahara/+bug/1534383

Cheers,

Aaron