Now that I’m finally on the right track I feel like I’ve made some good progress on the extend-able themes project. I’ve got a ‘working’ template loader built – there is still one problem with the loader getting stuck in an infinite loop, but it does load the correct templates at this point. I’m pretty sure I should be able to have this project completed in early March.
Time Line:
- Templates (1 week)
- Finish the extension template loader (1-2 days)
- Keep breaking up templates into smaller pieces (2-4 days)
- Test & Document (1 week)
So, working on the template loader – I’m finally on the right track with this project
All seems to be going well, I have it set up to try and load the extended version before going for the default template. But, there is a small catch… when the custom template extends the default template things get a bit messy. Because of the way the loaders are set up, the loader looking for extended templates always runs first when searching for any template. So, the new template loader looks for template_name + _extended each template load… this means that each time a template is loaded the _extended version is loaded first.
This at first seems like exactly what we want, but when the extended template extends the original… well, the new loader runs first and reloads the extended template – which extends the original, so reloads the extended version using the new loader, etc. So, it gets into an infinite loop of loading the same extended template.
I’m thinking that there would be a couple of ways to fix this:
- load from a special extended templates directory structure (but this could lead to the same problem, only in a different directory)
- have a set of default templates and used templates, then only extend the defaults (the regular templates would also extend the defaults, there just would be no changes – but then why not just have defaults & extensions?)
- know what template is calling the loader and if it is requesting the same template then don’t load using the extension loader (this seems to be the best & cleanest option to me as it gets rid of the need for extra template files and solves the infinite recursion problem – now, to figure out how to do this)
- or I could do this a bit backwards, have the special template loader remove the _extended part of the template_name and change all of the url redirects to template_name_extended – but this doesn’t seem very clean, it would keep the number of files down though
Project Milestone Plan
1 week: add in forms to the theme settings page (have this completed using charFields right now – just need to build the customized forms to swap in)
2 weeks? (I am really not sure about this one): build the color-selector form & the image selector form for the theme settings page
- Color selection component (point and click on a desired color)
- Banner image selector/loader
- Restore defaults for each editable component
- Restore all defaults option
1 week: create a model to store the default theme values and the current values (defaults if there has been no editing) & add them into the theme settings page instead of the CharFields
After that I’m not sure about time, but have a plan on how to actually implement the change in themes:
- Pull out all of the editable values from the commons.css files and create a new .css file called commons_editable.css (should not take long). Also, edit the templates to use this new .css file as well as the commons.css.
- Whenever the themes are changed (and the changes are saved) re-write the editable.css with the new current values from the models
- Will also want to make a validation system for the banner image (should we allow uploading a banner image, or just using a url?) and the color values (the user should be able to just input a hex value instead of being forced to use the color selector).
- Try to break it
- Fix problems and repeat 4-5 until I can’t break it anymore
If there is time for me to take on another small project (or if things go faster than anticipated) I can look into working on one of the extension projects (or something else).
So, I did some more work on issue 1632. I’m not sure how the order_by(sort_list) actually uses the sort list to generate the SQL code, but it seems like my initial attempt to fix the problem (adding lower() around the submitter sort_item) will not work. I took a look at the SQL code being generated with the query.query.__str__() and adding lower() to the sort list didn’t seem to change anything – it also broke reviewboard (lower(submitter) was not a valid field in the query).
Project:
I will be working on adding customizable themes by the admin as my first project for reviewboard. So, I spent the rest of the day figuring out how the .html files were actually being linked to the GUI. I managed to make a new theme tab under the admin page’s settings tab by using the storage tab’s .html template. This required adding in a new /style url to the urls.py under reviewboard/admin as well as altering the reviewboard/admin/views.py file by adding in a new class StyleSettingsForm.
Next:
I will be looking into how to actually use the django framework to populate the theme page with data and then figuring out what kind of things should be customizable/making the GUI.
Getting setup at the start of the day went pretty well (had already made a fork on github and had review board running on my machine). Getting git setup properly (cloning my fork and setting up a branch to develop on, and just trying it out) took a lot out of the morning. Following setup we moved on to looking at the easy-fix bug list.
Issue 1632:
I was able to reproduce issue 1632 using sqlite as my underlying database, so decided to take it on.
With help from Mike I was able to step through the code base to find approximately where this issue is being caused. I am new to using django however, and the problem seems to be a combination of an order_by() call on a queryset in the djiblets.datagrid.DataGrid class, and whether or not the database being used is case sensitive.
Some checking into the django API revealed that the order_by method’s case sensitivity depends on how the underlying database handles case. So, it looks like the generated SQL does not use ORDER BY LOWER(field) as a default way to handle upper/lower case orderings.
Possible solutions:
- add a new column of the lowercase submitter names so the sort could happen to the lowercase column instead (would only work for the submitter column – also storing redundant data)
Have tried:
- adding a “lower()” call around the submitter field in the sort_list in the precompute_object method in djiblet.djiblet.datagrids.datagrid.py (DataGrid class). I would think that adding this around some of the sort_list items (the strings) would make the order by call use ORDER BY LOWER(string1), LOWER(string2), …
So far this hasn’t worked (have to look into the database to see what field I actually want to use lower on (if that works then a regex could be used to handle all strings that way – assuming we want to sort all strings without case sensitivity)
Recent Comments