New branch with the toaster table being a subclass of the django View as discussed

 michaelw/toaster-tables-poc-v2

e.g.
http://127.0.0.1:8000/toastergui/two-tables-poc/
http://127.0.0.1:8000/toastergui/recipes-poc/
http://127.0.0.1:8000/toastergui/layers-poc/

Note: the data is all just sample queries for demonstration
known issue that the two-tables-poc rows selector changes both tables there are some duplicate ids that I need to fix

Michael

On 15/04/15 12:37, Michael Wood wrote:
On 14/04/15 16:52, Damian, Alexandru wrote:
Hi,

I've taken a look at the POC, looks pretty good - I have some comments below.

* I am not sure why you re-implement the casting of the correct table type instead of using language inheritance; I'm referring to:

        table_new = tables_available[table_name]
        table_kargs = {'project_id' :request.session['project_id']}
        table = table_new(**table_kargs)

...
tables_available = {
  'recipes' : RecipeTable,
  'layer' : LayersTable,
  # ...and so on
}

Instead of just directly sub-classing Table > ToasterTable > RecipeTable; This approach would ensure proper endpoints directly in the template view class, instead having to manually sync the /tables_available/ with the defined classes and magic keywords for the table name. Also, see next point.


Thanks yeah I think I see what you're saying, basically have ToasterTable subclass the django View so then each table will be a view. Negating the need to have a class in-between. I'll add this to the todo for a V2.


* I understand the need to have different endpoints for HTML and JSON returns; I would argue that these are just different views of the same object - the table object, also see the last point - and in REST fashion , they should have related URLs.

I.E instead of using /layers-poc// and /table-poc/layers/ as HTML/JSON endpoints, I would use /layers-poc// and /layers-poc/.json/ . This ties into the inheritance point above, as the Toaster.get() can be written to return HTML/JSON based on the calling endpoint.


I can see what you're saying, however I think that it's more important that we have smaller modules with single responsibility / logical cohesion for the two separate responsibilities of rendering a page vs API JSON response.

For example we have a page like the layerdetails page that loads the Machines table and Recipes table, the modules in that page you'll be accessing would have a whole load of code in them which is for rendering a page that is nothing to do with the layerdetails page.


* There is not clear to me how the cache invalidation happens. Considering that frequently we change the database data through the Bitbake interface, as opposed to the user interface, we need some sort of policy on cache invalidation.


It invalidates after a few seconds see https://docs.djangoproject.com/en/1.7/topics/cache/#django.core.cache.caches

It was just an example of something that we could look further into.



* I like the concept of using AJAX to refresh the table content; I am a bit concerned about lack of graceful degradation, e.g. what the user sees if there is no JS enabled on the machine. I would suggest rendering the initial data content (in some way) in HTML and enable minimal pagination/search/interaction links to run using classic HTML href requests (this implementation needs to provide minimum navigation, and does not need be complete); the layer using AJAX to refresh the data should come on top of the basic functionality - e.g. change the links from HTML href to javascript actions - as to provide the better experience if JS is enabled; but we shouldn't penalize our users for not using JS.


I don't see a need to support no javascript enabled users. Toaster currently requires Javascript so nothing changes here.


I'm not touching minor issues as this is clearly a POC for opening up the discussion and not a ready-to-drop replacement.


Thank you and keep up the impressive work that went into this !

Cheers,
Alex

On Fri, Mar 27, 2015 at 1:51 PM, Michael Wood <[email protected] <mailto:[email protected]>> wrote:

    On 27/03/15 11:44, Barros Pena, Belen wrote:

        Hi Michael,

        Thanks for this. I have a couple of questions, if that's ok.

        1. Can you share some context about what is prompting this
        refactoring of
        the front-end code? I know the background because I talk to
        you all the
        time, but it might be useful to other people on the list.


    Sure,

    Currently we have a collection of defined ways to describe a table
    in Toaster on the back end and the front end.
    The first purpose is to consolidate this into a single object on
    the back-end and a single handler on the front end, doing this
    will hopefully mean fewer bugs and consistency in behaviour and
    look across all the tables.

    It also allows us to add features that can then be utilised across
    all tables, such as the one included in this patch which is an
    improvement to the user experience by avoiding having to do a page
    reload for every table operation. I also think it improves the
    speed of the page loading because we're only exposing the fields
    from the database that we require rather than an object that
    represents all the data and has an easy way to implement caching.


        2. Can you provide more details about the "easy switch path"?
        :) When
would be a good time to do it, what kind of extra QA work would be
        required (if any), etc

    Don't think there would be any extra QA work. If anything it may
    be simpler because we can write some tests that check the JSON
    output. The easy switch path is that it can be done gradually if
    we want. The best time would probably be after 1.8 -- nothing
    stopping us branching and starting now.


        3. Do you think this approach can be expanded to the other UI
        components
        (filters, sorting, edit values in forms, layer dependencies
        modal dialogs,
        type ahead, etc)? Do you have a full list of what those
        components would
        be? How much work / time will it take to create them and
        making the
        switch, etc...

    No I don't have a list, (filters will be part of the table work I
    just didn't implement them for the POC) It'd be good to make one,
    then a better time estimate could be made. It took around 4 days
    to do the table stuff, but that included a lot of experimenting
    and reading around it.

    Yes I think we could expand this approach to other components, we
    have a few done already on the client side; typeahead and the
    layer dependency modal mechanism.
    We would need to do an audit and workout what other common
    components could be done.



        Sorry for all the questions: I am keen on doing this so that
        we have a
        solid foundation for the impending feature creep, but I am
        having some
        trouble picturing how much time / effort is involved.

        Thanks!

        Belén

        On 26/03/2015 18:21, "Michael Wood" <[email protected]
        <mailto:[email protected]>> wrote:

            Here is the current WIP of the proof of concept of the
            Tables widget for
            toaster

http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=michaelw/toa
            ster-tables-poc
            ( http://tinyurl.com/op63pgj )


            I've tried to prefix/postfix 'poc' to make it easier to
            identify which
            are for the proof of concept

            Brief overview of the files/changes:

            ----------------------
            poc_view.py:

            Table(View):
                 - Handles the HTTP requests to get the table data
                 - Adds some basic caching (future - this can be tied
            into the
            request but for now is just global)


            ToasterTable(object):
                 - Adds a base class for common table functionality
                 - A lot of this is bringing in all the parts we
            already have in
            views.py and standardising them in a single class
                 - In many tables we have what I've named "static
            data" this is
            where we have a column that contains html data, for
            example a button.
                 - Outputs the data for the page requested as a JSON
            document


            RecipeTable(ToasterTable):
                 - Example of a table which has static data

            LayersTable(ToasterTable):
                 - Example of a simple table

            ----------------------

            tables.js:
                 - This consumes the table data
                 - Adds handlers for all the table chrome buttons
                 - Implements the add/remove columns
                 - Implements paging

            ----------------------

            table-poc.html:
                 - Template that just contains the html for the table
            layout (i.e.
            search bar, table, buttons etc)
                 - can be considered the same as conflating the
            "basetable_top.html"
            + "basetable_bottom.html" templates

            ----------------------

            generic-table-page-poc.html:
                 - This is a normal table page in toaster with
            navigation, zone
            alert and which includes the table-poc.html template the
            context
            variable table_name is passed into this template to select
            which table
            to display

            ----------------------

            two-tables-poc.html:
                 - Exactly the same as generic-table-page-poc.html
            (above) but has
            two tables in it.

            ----------------------

            changes to urls.py:
                 - Add the poc-table this calls into to Table(View) to
            handle the
            requests made from the ajax calls (could be renamed
            xhr_table for
            consistency)
                 - Add urls for the test pages: /layers-poc/ and
            /recipes-poc/ and
            /two-tables-poc/ note that they don't need their own views
            defined
            because the only thing changing is the table_name and
            that's passed in.

            ----------------------

            Notes:

            The whole lot is 642 lines which means the maintenance
            burden shouldn't
            be too high
            There's a fairly easy path to switching our existing
            tables - one at a
            time if needed
            There are bugs and features missing - This is just the
            proof of concept!

            Thanks,

            Michael









            -- _______________________________________________
            toaster mailing list
            [email protected] <mailto:[email protected]>
            https://lists.yoctoproject.org/listinfo/toaster


    --     _______________________________________________
    toaster mailing list
    [email protected] <mailto:[email protected]>
    https://lists.yoctoproject.org/listinfo/toaster




--
Alex Damian
Yocto Project
SSG / OTC

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



--
_______________________________________________
toaster mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/toaster

Reply via email to