Leap2A import isn't populating the artefact_file_embedded table for most things

Bug #1512894 reported by Aaron Wells
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Confirmed
Low
Unassigned

Bug Description

The artefact_file_embedded table was added with the TinyMCE image picker button. Its purpose is to store the embed relationship, so that it can be checked for permission purposes (I have permission to view this thing; do I then have permission to view this other thing?)

Currently, it's somewhat redundant for some purposes, because this relationship is also tracked in the view_artefact & artefact_attachment tables, as well as a couple of other places. But artefact_file_embedded is unique in that it's abstract enough to cover cases that aren't covered by the other tables, such as view descriptions and forum posts.

So, for best practice, we should always be populating this table whenever we embed an image into a text field. And we do this pretty much everywhere *except* in Leap2a imports.

For Leap2a imports, artefact_file_embedded is populated for images in view descriptions, but not for images embedded in anything else: blog entries, resume fields, text blocks, notes, etc.

We should re-use the EmbeddedImage->rewrite_embedded_image_urls_from_import() method, for all imports. By my analysis, it can probably go in (or maybe replace) the call to ImportLeap->fix_artefact_references() in ImportLeap->do_import()

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Low priority, because the current redundancy of the view_artefact table means there are no visible side-effects to the artefact_file_embedded table not getting populated.

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Actually, it turns out this does cause problems for embedded images in the Profile "Introduction" field, because that relies exclusively on artefact_file_embedded for viewing permission.

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Spun off bug 1521368 about the profile introduction issue specifically.

tags: added: export import leap
removed: leap2a
Revision history for this message
Aaron Wells (u-aaronw) wrote :

Reposting this comment, which I originally made on Bug 1520328:

In fixing this, it should be noted that you also need to test for whether the embedded image is visible to other users:

1. Share the page with another user (or with "Public" or "Registered users")
2. View the page as another user and see if the image displays properly.

A user's permission to view a page is currently tracked via several different tables, at least one of which must have an appropriate record:

 - artefact_attachment: For classic "attachments", like those on notes, blog entries, and comments
 - view_artefact: For artefacts displayed directly by selection in a block (e.g. the image block)
 - artefact_file_embedded: For images embedded in another artefact's rich text fields

The import process must populate any of these that are appropriate, in order for it to work correctly.

Aaron Wells (u-aaronw)
no longer affects: mahara/16.10
Changed in mahara:
milestone: 16.04.0 → 16.10.0
Robert Lyon (robertl-9)
Changed in mahara:
milestone: 16.10.0 → 16.10.1
Robert Lyon (robertl-9)
Changed in mahara:
milestone: 16.10.1 → 17.04.0
Changed in mahara:
milestone: 17.04.0 → none
Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

The embedded image problem was resolved along the way and I couldn't replicate it anymore.

For the rest, the code needs to be reviewed and investigated if it is still an issue.

tags: added: review
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.