View layout becomes corrupted when changed

Bug #1537861 reported by Matt Di Giuseppe
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Robert Lyon
16.04
Fix Released
High
Unassigned
16.10
Fix Released
High
Robert Lyon

Bug Description

Version: Mahara 15.04.5 (only encountered on this version, after upgrading from 15.04.4)

We have had 2 users (out of over a thousand) have this problem so far, and I've been unable reproduce it or trace a specific cause but we do know the order of events and the outcome:

They copy a page from another user
Change content or add some new content
Attempt to change the layout (say from 3 columns to 2 columns)

Something goes wrong while the layout is being changed and the data in the table view_rows_columns no longer matches what's in the view table, and the page won't load anymore.

As more users encounter this problem (and I'm assuming they will) I will update this report.

Tags: layout view
Revision history for this message
Matt Di Giuseppe (matthew-digiuseppe) wrote :

I got a description from a user, which doesn't make a lot of sense in this context:

"I did change the layout of the page, but after I made that change and saved it, everything
was still working perfectly. I put a few ideas of my own onto the page by using the text block feature, which also had zero issues. The last thing I did before saving my work and logging out was add my Oxford University Journal Article. I added it in pdf format, and then clicked done at the bottom of the page; everything looked great. The problem arose about 45 minutes later, when I logged back in to continue my work, and it refused to load the page."

So it looks like something happened to the page while the user was not editing it, which makes no sense to me unless shared pages are possibly retaining links to the layout of the original?

Another user had the same problem today, which I can give some more information on. The page SHOULD have 6 rows and 2 columns according to the view table. In the views_rows_columns table it only has entries for rows 3-6, and each row only has 1 column. view_layout agrees that there are 6 columns. view_layout_columns also indicates 6 rows, but only 1 column each.

So there's a discrepancy between view and pretty much everything else, and in view_rows_columns where entries for the first 2 rows are missing.

On a related note, why are there so many tables to track layout?

Revision history for this message
Matt Di Giuseppe (matthew-digiuseppe) wrote :

To fix the above problem I had to add entries for rows 1 & 2 in view_rows_columns, and change each entry to have 2 columns (50,50) in that table, as well as in view_layout_rows_columns.

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/5974

Revision history for this message
Matt Di Giuseppe (matthew-digiuseppe) wrote :

And here's a new, possibly related layout problem. I have a shared page that is displaying as a 2 column, 6 row layout, but the 2nd column doesn't exist to drop artifacts into. I can only drop artifacts into the first column (any row).

I tried to change it to a one column layout and got an unrecoverable error. I tried a 2 column layout and it said it couldn't change the layout because another user may also be editing the layout. I tried a 3 column layout and it converted itself back to 1 column.

Revision history for this message
Matt Di Giuseppe (matthew-digiuseppe) wrote :

The unrecoverable error when trying to convert to 1 column was "You cannot remove the last column from this page." Obviously, it didn't exist.

Changed in mahara:
status: New → In Progress
milestone: none → 16.04.0
importance: Undecided → High
assignee: nobody → Robert Lyon (robertl-9)
Revision history for this message
Aaron Wells (u-aaronw) wrote :

Hi Matt,

In belated response to your question about why there are so many tables to track view layouts, we do have a couple that could be consider unnecessary if everything was redesigned, but on the whole it's actually a fairly logical mapping of the flexibility of our page layout engine into a relational database:

1. view_layout_columns: A "column layout", i.e. a number of columns, and what page-width each one has. (1 column, 2 columns 50/50, 2 columns 67/33, etc). These are hard-coded.

2. view_layout: Each record is a view layout (i.e. a combination of rows and columns). This includes the 34 hard-coded view layouts that ship with Mahara, and any custom layouts users have produced (if my math is correct, with 1 to 5 rows and 12 different column layouts, there are 12^5 + 12^4 + ... + 12 = 271,452 possible layouts)

3. view_layout_rows_columns: Each record represents one row in a view layout. It indicates which row number it is, and which of the column layouts it uses.

4. usr_custom_layout: Each record is one custom layout generated by a user (or group or institution). This table is used to "bookmark" previously used custom layouts for the user.

If I was rewriting it from scratch, I would probably rename some of the tables to make things clearer. The relationship between view_layout, view_layout_columns, and view_layout_rows_columns is not really made clear by the table names or their column names.

Also, since we've currently hard-coded the views to a maximum of 5 rows, it could also make sense to eliminate the view_layout_rows_columns table, and instead give view_layout 5 fields called "row1layout", "row2layout", etc. And from there, you could also eliminate the "view_layout_columns" table, which describes the column layout with strings like "33,33,33", and just move those strings directly into view_layout.row1layout, view_layout.row2layout, etc.

But, at this point it's probably not worth all the refactoring that would require.

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

Patch 5974 (remove vestigial database field view.numcolumns) doesn't actually address this bug, so I'm moving it to a separate Bug report: Bug 1557825.

Changed in mahara:
status: In Progress → Confirmed
Revision history for this message
Robert Lyon (robertl-9) wrote :

Another issue with the view layout system is the use of the label 'columns' on some of the layout related tables.

In view_rows_columns the 'columns' represent the count of the number of columns in the row

But in view_layout_rows_columns the 'columns' represent an id that maps to the view_layout_columns table where 'columns' also represents an id for the different types of widths

It would be worth cleaning this up so that the 'columns' that represent an id be changed to something like 'columnid' so to differentiate the id ones from the count ones

Robert Lyon (robertl-9)
Changed in mahara:
milestone: 16.04.0 → 16.04.1
Robert Lyon (robertl-9)
Changed in mahara:
milestone: 16.04.1 → 16.04.2
Robert Lyon (robertl-9)
Changed in mahara:
milestone: 16.04.2 → 16.04.3
Revision history for this message
Robert Lyon (robertl-9) wrote :

This should now be fixed by https://reviews.mahara.org/#/c/7184/

Changed in mahara:
milestone: 16.10.1 → 17.04.0
status: Triaged → Fix Committed
no longer affects: mahara/15.04
no longer affects: mahara/15.10
Changed in mahara:
status: Fix Committed → Fix Released
milestone: 17.04.0 → none
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.