level12 / keg-elements Goto Github PK

Oull integration ideas from IntegrityHR and FISResid.

• Users
• Groups (groups Roles)
• Roles (groups Permissions)
• Permissions

pytest isn't going to support yield

projects/keg-demo-repo/kegdemo/tests/test_model.py::TestBlog::()::test_column_checks
  yield tests are deprecated, and scheduled to be removed in pytest 4.0

projects/keg-demo-repo/kegdemo/tests/test_model.py::TestComment::()::test_column_checks
  yield tests are deprecated, and scheduled to be removed in pytest 4.0

projects/keg-demo-repo/kegdemo/tests/test_model.py::TestUser::()::test_column_checks
  yield tests are deprecated, and scheduled to be removed in pytest 4.0

-- Docs: http://doc.pytest.org/en/latest/warnings.html

I have this function which helps create a non-nullalbe field in migrations. Something we do often and is the same thing over and over again. Create the field as nullable, set a default value, alter the column to a non-nullable.

This is completely self-contained besides the import. Putting it somewhere is the tricky part.

• You can't really put it into the alembic folder since importing (from . import utils or import utils) messes with how Alembic gets the configuration or namespaced messes with import alembic.
• Changing the alembic scripts folder is an option to prevent namespace pollution but that turned out to be just as messy and requires a bunch of changes that aren't ideal.
• The best option is to have it in a separate library. I was thinking keg_elements.alembic might be a good spot or maybe keg_elements.migrations.utils I am not sure what namespace, but somewhere we can keep helper scripts like this as a part of keg_elements.
• Final option (second best), I create a new python package and we keep things more organized.

import copy
def new_non_nullable_field(op, table_list, column, default_value=0):
    """Create a new non-nullable field in the list of tables

    :param op: the operational module or batch operator
    :param table_list: a list of two item tuples (schema, table_name)
    :param column: the SQLAlchemy column declaration
    :param default_value: the value to set existing fields
    :returns: None
    """

    if type(default_value) in [str, unicode]:
        default_value = "'{}'".format(default_value)

    for schema, table in table_list:

        # Copy the column to avoid polution by SA operations
        col = copy.copy(column)

        op.add_column(table, col, schema=schema)

        op.execute("""
        UPDATE {schema}.{table}
        SET {col_name} = {default}
        """.format(table=table, col_name=column.name, schema=schema, default=default_value))

        op.alter_column(table, column.name, schema=schema, nullable=False)

At least one client is using this in Python 3.5 so it would be nice to start running tests on it.

We have a FormBase testing class that we copy often enough from project to project. Here are some examples:

• https://github.com/level12/evolve-airtonomy/blob/69fcdba636448fccce91adf9506c4440feb35da4/webapp/airtonomy/libs/testing.py#L235
• https://github.com/level12/moleliminator/blob/26611edadad6d520a32b1ece861e59ffb1fce30c/moleliminator/libs/testing.py#L212
• https://github.com/level12/fis-climate/blob/7c2b5914d756b7354863be1b6b6bfb9a3353f041/climate/tests/utils/bases.py#L24

This class should probably be moved into one of our libraries. This seemed like the most relevant repo to me, but I hold that opinion loosely, so feel free to move this issue around.

1.0 support will end on 2018-08-31

The current form template API does not provide any mechanism to disable the Cancel link. This is a problem because sometimes a form does not have a logical place to go after the user clicks Cancel. In such cases it would be better to simply not have a Cancel link. We already have several forms in production whose Cancel links take you back to the same form you were on before.

For testing it would be helpful to have the field name on the div.labeled-group so you can select the help-box easily.

In one of our projects we have a handy LookupMixin for entities which are just labels. Should we port that into KegEl?

I would use it in my permissions extension if it were available in KegElements.

I switched these to utc time, but I'm not sure how to make a server_default that works on all DBs we support. I asked a question on the SA mailing list about this:

https://groups.google.com/forum/#!topic/sqlalchemy/NpK5n59QbV8

• Fix check box input https://github.com/level12/keg-elements/blob/master/keg_elements/templates/keg_elements/forms/horizontal_b4.html#L166 needs a custom-control-input even when errors exist.

CollectionUpdater.find_child() will only ever check the first element in the collection for an existing record.
https://github.com/level12/keg-elements/blob/master/keg_elements/db/utils.py#L130

The form generator should automatically exclude created_utc and updated_utc when they are provided by the DefaultColsMixin

WTForms does not support form-level validators or form-level errors. To support these, we'll need to extend WTForms somehow. A workaround is suggested here.

• The tests seem to fail randomly, but upon further review they seem to fail on any fresh installation. By removing recreate=false in the tox configuration, the failures go away.

• The main issue is the None type doesn't have __dict__, the other two errors seem to be related or directly caused by that one.

• I was, at one point, able to get the tests to pass in 3.8 with minimal changes, but they then failed on 3.6... whack a mole.

• By adding a simple delay to the test, it seemed to fix the issue, but then it popped back up again.

Would it be nice if the form generator recognized required foreign keys, like Payee.sales_rep_id and also set the coerce type to int to match?

https://github.com/level12/fis-climate/pull/1540/files

We match on field type in the template, but should also check the widget type.

Automatic python form validation does not include checking the number of decimal places on a Numeric data type.

• Somehow test that dynamic/static templates offer the same interface.
• Test that static templates ignore CSRF fields entirely.
• Test that static templates render radio fields properly.

Currently, the check_column_fk ColumnCheck does not support columns which are foreign keys to multiple tables.

Add CI support for Windows.

Right now, we have:

• Form.errors: field errors
• Form.form_errors: form level errors
• Form.all_errors: form & field level errors

I think it would make more sense to have:

• Form.field_errors: field errors
• Form.form_errors: form level errors
• Form.errors: form & field level errors

As reported in level12/fis-residuals#49 we need a way to indicate that a field is required.

I am thinking for now something as easy as a red * like we do in Blazeweb?

When using the choices_modifier on a FieldMeta we have seen a case where the choices are present for the first page load and then gone for the second page load. Clearly something is being mutated that should not be.

Child objects that have a unique constraint on a non-PK field cannot be replaced with another object with the same value in the unique field. The add() and edit() methods cause the collection to be flushed before the records that aren't in the new data list are removed resulting in an uncaught integrity exception.

To fix this issue, I think we would need do the remove step first before adding and editing the remaining records. It may also be a good idea to disable flushing for the add() and edit() calls and flush once before returning to avoid possibly accidentally triggering other constraints.

There should be a base testing class for testing webrid grids. We use the following in one of our projects:

import re
import urllib.parse

from blazeutils.spreadsheets import workbook_to_reader
import flask
import flask_login
from pyquery import PyQuery
import pytest
import sqlalchemy


def query_to_str(statement, bind=None):
    """This function is copied directly from sqlalchemybwc.lib.testing

        returns a string of a sqlalchemy.orm.Query with parameters bound
        WARNING: this is dangerous and ONLY for testing, executing the results
        of this function can result in an SQL Injection attack.
    """
    if isinstance(statement, sqlalchemy.orm.Query):
        if bind is None:
            bind = statement.session.get_bind()
        statement = statement.statement
    elif bind is None:
        bind = statement.bind

    if bind is None:
        raise Exception('bind param (engine or connection object) required when using with an '
                        'unbound statement')

    dialect = bind.dialect
    compiler = statement._compiler(dialect)

    class LiteralCompiler(compiler.__class__):
        def visit_bindparam(
                self, bindparam, within_columns_clause=False,
                literal_binds=False, **kwargs
        ):
            return super(LiteralCompiler, self).render_literal_bindparam(
                bindparam, within_columns_clause=within_columns_clause,
                literal_binds=literal_binds, **kwargs
            )

    compiler = LiteralCompiler(dialect, statement)
    return 'TESTING ONLY BIND: ' + compiler.process(statement)


class GridBase(object):
    grid_cls = None
    filters = ()
    sort_tests = ()

    @classmethod
    def setup_class(cls):
        cls.user = User.testing_create()

        if hasattr(cls, 'init'):
            cls.init()

    def assert_in_query(self, look_for, **kwargs):
        pg = self.get_session_grid(**kwargs)
        query_str = query_to_str(pg.build_query())
        assert look_for in query_str, '"{0}" not found in: {1}'.format(look_for, query_str)

    def assert_not_in_query(self, look_for, **kwargs):
        pg = self.get_session_grid(**kwargs)
        query_str = query_to_str(pg.build_query())
        assert look_for not in query_str, '"{0}" found in: {1}'.format(look_for, query_str)

    def assert_regex_in_query(self, look_for, **kwargs):
        pg = self.get_session_grid(**kwargs)
        query_str = query_to_str(pg.build_query())

        if hasattr(look_for, 'search'):
            assert look_for.search(query_str), \
                '"{0}" not found in: {1}'.format(look_for.pattern, query_str)
        else:
            assert re.search(look_for, query_str), \
                '"{0}" not found in: {1}'.format(look_for, query_str)

    def get_session_grid(self, *args, **kwargs):
        flask_login.login_user(kwargs.pop('user', self.user), force=True)
        g = self.grid_cls(*args, **kwargs)
        g.apply_qs_args()
        return g

    def get_pyq(self, grid=None, **kwargs):
        pg = grid or self.get_session_grid(**kwargs)
        html = pg.html()
        return PyQuery('<html>{0}</html>'.format(html))

    def get_sheet(self, grid=None, **kwargs):
        pg = grid or self.get_session_grid(**kwargs)
        xls = pg.xls()
        return workbook_to_reader(xls).sheet_by_index(0)

    def check_filter(self, name, op, value, expected):
        qs_args = [('op({0})'.format(name), op)]
        if isinstance(value, (list, tuple)):
            for v in value:
                qs_args.append(('v1({0})'.format(name), v))
        else:
            qs_args.append(('v1({0})'.format(name), value))

        def sub_func(ex):
            url = '/?' + urllib.parse.urlencode(qs_args)
            with flask.current_app.test_request_context(url):
                if isinstance(ex, re.compile('').__class__):
                    self.assert_regex_in_query(ex)
                else:
                    self.assert_in_query(ex)
                self.get_pyq()  # ensures the query executes and the grid renders without error

        def page_func():
            url = '/?' + urllib.parse.urlencode([('onpage', 2), ('perpage', 1), *qs_args])
            with flask.current_app.test_request_context(url):
                pg = self.get_session_grid()
                if pg.page_count > 1:
                    self.get_pyq()

        if self.grid_cls.pager_on:
            page_func()

        return sub_func(expected)

    def test_filters(self):
        if callable(self.filters):
            cases = self.filters()
        else:
            cases = self.filters
        for name, op, value, expected in cases:
            self.check_filter(name, op, value, expected)

    def check_sort(self, k, ex, asc):
        if not asc:
            k = '-' + k
        d = {'sort1': k}

        def sub_func():
            with flask.current_app.test_request_context('/?' + urllib.parse.urlencode(d)):
                self.assert_in_query('ORDER BY %s%s' % (ex, '' if asc else ' DESC'))
                self.get_pyq()  # ensures the query executes and the grid renders without error

        def page_func():
            url = '/?' + urllib.parse.urlencode({'sort1': k, 'onpage': 2, 'perpage': 1})
            with flask.current_app.test_request_context(url):
                pg = self.get_session_grid()
                if pg.page_count > 1:
                    self.get_pyq()

        if self.grid_cls.pager_on:
            page_func()

        return sub_func()

    @pytest.mark.parametrize('asc', [True, False])
    def test_sort(self, asc):
        for col, expect in self.sort_tests:
            self.check_sort(col, expect, asc)

    def assert_table(self, table, grid=None, **kwargs):
        d = self.get_pyq(grid, **kwargs)

        assert len(d.find('table.records thead th')) == len(table[0])
        for idx, val in enumerate(table[0]):
            assert d.find('table.records thead th').eq(idx).text() == val

        assert len(d.find('table.records tbody tr')) == len(table[1:])
        for row_idx, row in enumerate(table[1:]):
            len(d.find('table.records tbody tr').eq(row_idx)('td')) == len(row)
            for col_idx, val in enumerate(row):
                read = d.find('table.records tbody tr').eq(row_idx)('td').eq(col_idx).text()
                assert read == val, 'row {} col {} {} != {}'.format(row_idx, col_idx, read, val)

    def expect_table_contents(self, expect, grid=None, **kwargs):
        d = self.get_pyq(grid, **kwargs)
        assert len(d.find('table.records tbody tr')) == len(expect)

        for row_idx, row in enumerate(expect):
            td = d.find('table.records tbody tr').eq(row_idx).find('td')
            assert len(td) == len(row)
            for col_idx, val in enumerate(row):
                assert td.eq(col_idx).text() == val

The current default is arrow.now, should be arrow.utcnow. Should not require a migration since the server default and column types are correct.

I am specifically talking about how to deal with HTTP status codes, when to redirect, with which code.

Situations

• Logged In
  • Authorized to view requested page
  • Not authorized to view requested page
• Logged Out

Potential Codes

• 301 (Moved Permanently)
• 302 (Found)
• 303 (See Other)
• 307 (Temporary Redirect)
• 308 (Permanent Redirect)
• 401 (Unauthorized)
• 403 (Forbidden)

Action

• Redirect to login
• Render login page without redirect
• Show error page
• Other thing?

@nZac is there a reason you pinned everything in this file? https://github.com/level12/keg-elements/blob/master/Pipfile

And why are you ignoring the lock file? c97dcc8

Thanks.

Look at Keg Storage as the example. Submit to Team B / NZ for review.

FlaskWTFDeprecationWarning: "flask_wtf.Form" has been renamed to "FlaskForm" and will be removed in 1.0.

Currently, the tab indexes can be added to a form field individually. This issue will address adding tab indexes when we render a wtform using a jinja macro instead of rending individual form fields.

Example:

class Location(db.Model):
    latitude = sa.Column(sa.Numeric(11, 8),  info=dict(randomdata=random_latitude))
    longitude = sa.Column(sa.Numeric(11, 8),  info=dict(randomdata=random_longitude))

Where random_latitude and random_longitude can be callables returning randomized data appropriate for their specific column.

testing_create should check for Location.latitude.info['randomdata'] first and fallback to the type based default if a custom randomdata method is not available

A field with a description is now broken into two columns, which looks bad when everything else is full-width.

This would be better served as an input group append, in line with the label, or by merely displaying the description by default below the form element as info text which has the added benefit of not requiring JS.

wtforms does this already with Meta. For example:

class FormA(Form):
    class Meta:
        a = 'foo'

class FormB(FormA):
    class Meta:
        b = 'bar'

form = FormB()
print(form.meta.a)  # 'foo'
print(form.meta.b)  # 'bar'

See https://github.com/wtforms/wtforms/blob/23f730a9cfca478f01fda2b38fde17ad56e9a83d/src/wtforms/form.py#L193
and https://github.com/wtforms/wtforms/blob/23f730a9cfca478f01fda2b38fde17ad56e9a83d/src/wtforms/form.py#L259

Right now ValidateUnique requires that the Form have a obj member. This must be added manually to the form since it's not included by default on a WTForm form. However, WTForms do have a _obj member which has the same purpose. We could make ValidateUnique easier to use by utilizing this member.

Currently there is no validator for alphanumeric (Characters and numbers only). It would be nice to have one for reuse.

It seems to create validators for a ModelForm any Numeric columns on the model are assumed to have both precision and scale set: (focus on lines 367-368)

I was setting up a ModelForm for a model with a Numeric column on which precision and scale were not set. It failed like this:

keg_elements/forms/__init__.py:315: in _max_for_numeric
    return Decimal('{}.{}'.format('9' * (digits - scale), '9' * scale))
E   TypeError: unsupported operand type(s) for -: 'NoneType' and 'NoneType'

In my mind there are two possible directions to go with a fix for this:

1. We still want to require Numeric columns to have precision and scale set in order for us to generate validators for that form field. In which case, we should probably assert that precision and scale are set, and if they aren't give a more helpful error message.
2. We do not want to require Numeric columns to have precision and scale set. In which case, we should probably not call _max_for_numeric if either value is missing.

If we remove the automatic call to render_hidden we could avoid the additional complexity of filtering out hidden fields and just rely on our own macros to render the fields.

Care will need to be taken to make this change since this may cause problems where we use the block form of the macro or explicitly provide field names since the CSRF token field will no longer be rendered automatically in those cases.

This applies to both the horizontal.html and horizontal_b4.html.

• Numeric values should get random numbers instead of 0.
• Actual numeric types should get a number with a fractional part (the longer the better so we can more easily detect places where formatting needs to be applied for display).

Add a US states dropdown with support for two letter values, full name values, two letter text, full name text.

Might be nice to add this as a custom SQLA field with enum support.

So you can have multiple forms on the same page and know which one got submitted.

I believe #56 broke the ability to use _commit and _flush in testing_create().

Traceback (most recent call last):
  File "/home/rsyring/projects/racebetter-src/racebetter/tests/test_toteupdates.py", line 60, in test_open_race
    race_ent = ents.Race.testing_create(race_status='Closed')
  File "/home/rsyring/projects/racebetter-src/racebetter/model/entities.py", line 93, in testing_create
    kwargs['event'] = Event.testing_create(_commit=False)
  File "/home/rsyring/.virtualenvs/racebetter/lib/python3.5/site-packages/keg_elements/db/mixins.py", line 217, in testing_create
    sorted(extra_kwargs))
AssertionError: Unknown column or relationship names in kwargs: ['_commit']

Reverting to KegElements 0.4.1 resolved my issue.