[#2224] - FIX REGRESSION In Cpanel modules widths due to buggy sample data set
- Closed
- 12 Oct 2013
- Medium
- Build: master
- # 2224
- Diff
- beat:patch-1
- Error The Travis CI build could not complete due to an error Details
User tests: Successful: Unsuccessful:
https://github.com/joomla/joomla-cms/pull/2206/files#diff-8fb2f5a83bfb0c557be664644876f1adL102
has fixed @brianteeman has only fixed this regression bug for new sites. This fixes it for upgraded sites too.
Needs a test as I did this on github to help, without time to test right now. Additionally github seems to bug on mergestatus-check! So really test it before committing please.
The sample data was buggy and @brianteeman fixed the buggy dataset with bootstrap-width of 1 instead of the default 0 in #2206 (without tracker item linked nor 2 @tests procedure).
The parameter bootstrap-width existed in those modules, but had a bug in implementation that got fixed.
So we need to do 1 of those:
- fix existing sites with that param set to 1 due to buggy sample dataset as proposed here.
- remove the "Bootstrap width" parameter entirely from the module parameters and remove its implementation here (which will not allow e.g. 2 or 3 or more columns of modules in backend cpanel !)
- rename the parameter to something not bootstrap-specific, e.g. "12ths of module width" and a new name with default of 12 and values of 1 to 12.
I think that "2" would be a pitty.
Poposal:
"1" is a quick fix for beta 1 here, and we could aim for fix "3" in beta 2.
Thoughts ?
Why not just update the respective entry in the database using the update SQL file? It should be possible when using proper where clauses which check the id and the params set by default. It only fails if the user did some changes to the params.
You can't do 2 or 3 as it would break templates which already use this param.
@Bakual Yes, that's the solution here, to add that in the SQL upgrade script, same as @brianteeman did in #2206 for new installations.
Can you please do a corresponding PR for the SQL upgrade scripts so I can close this one ?
Is there already a tracker for this? If not I can do one. Just didn't found one yet.
@Bakual Yes, #2231 works for me.
I didn't have time to add a tracker item for this Friday, and I actually did this quick PR as a very quick reply to a message on the bugssquad for a quick-fix for beta 1.
I'm now closing this PR in favor of #2231 as you'll have enough time to complete it for beta 2.
Thanks again for the good idea and the other PR (and tracker).
The sample data set is not buggy, it is an example of existing data faced with a backward compatibility breaking change. I'm not sure where this buggy concept is coming from.