Upgraded site doesn't rotate images

Bug #1760970 reported by Kristina Hoeppner on 2018-04-03
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
High
Robert Lyon
18.04
High
Robert Lyon
18.10
High
Robert Lyon

Bug Description

When you upgrade from 17.10 to 18.04RC2 / master, images that had been uploaded prior to the upgrade cannot be rotated.

Robert tracked it down to the database in the table "artefact_file_image". There it says:

orientation character varying(10) DEFAULT '0'::character varying,

and

CONSTRAINT artefileimag_ori_ck CHECK (orientation::text = ANY (ARRAY['0'::character varying

when on a fresh install it says "bigint" instead of "character varying".

Robert Lyon (robertl-9) wrote :

Turns out the problem has nothing to do with the postgres install vs upgrade db settings as there is already code to handle that.

The problem comes from this chain of events:

1) Install an older site (I practised with an old 16.10 site) and add some files including images via Content -> Files then check database table:
 SELECT * FROM artefact_file_image;

You should see the width/height values for the files

2) Upgrade to master and log in so you are on dashboard page then check database table:
 SELECT * FROM artefact_file_image;

You should see new column added with orientation 0 for the files + the width/height

3) Go to the Content -> Files page then check database table:
 SELECT * FROM artefact_file_image;

You should see all image files that exist in the home folder now have their width/height/orientation removed

The problem stems from the fact the files on upgrade are missing their artefact_file_files contenthash value and there is a bit of code in the ArtefactTypeFile class that checks to see if the value exists and if not generates it and commits the artefact.

And the problem comes in when we create a new ArtefactTypeImage instance we call the parent ArtefactTypeFile and commit it before we have full constructed the ArtefactTypeImage and so data is lost.

Because the parent Artefact class has a destructor we shouldn't need to explicitly call the commit() during construction of ArtefactTypeFile on line 1115 of artefact/file/lib.php because the it will be committed on destruction as long as we set the $this->dirty = true;

Reviewed: https://reviews.mahara.org/8768
Committed: https://git.mahara.org/mahara/mahara/commit/996f06f7ed3540e692687d4db9f318364cbcd9a3
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 996f06f7ed3540e692687d4db9f318364cbcd9a3
Author: Robert Lyon <email address hidden>
Date: Wed Apr 4 10:43:02 2018 +1200

Bug 1760970: Allow the artefact classes to commit in correct order

When the contenthash is missing

Also set the new orientation column as not null

behatnotneeded

Change-Id: I64de51a2a0f456ac54f7a47cbc6398fb1b588594
Signed-off-by: Robert Lyon <email address hidden>

Reviewed: https://reviews.mahara.org/8776
Committed: https://git.mahara.org/mahara/mahara/commit/ac1eab1158508adea3bb120d21aa9bf3cbc0b8e9
Submitter: Robert Lyon (<email address hidden>)
Branch: 18.04_STABLE

commit ac1eab1158508adea3bb120d21aa9bf3cbc0b8e9
Author: Robert Lyon <email address hidden>
Date: Wed Apr 4 10:43:02 2018 +1200

Bug 1760970: Allow the artefact classes to commit in correct order

When the contenthash is missing

Also set the new orientation column as not null

behatnotneeded

Change-Id: I64de51a2a0f456ac54f7a47cbc6398fb1b588594
Signed-off-by: Robert Lyon <email address hidden>
(cherry picked from commit 996f06f7ed3540e692687d4db9f318364cbcd9a3)

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers