teknik-eksjo / chronos

A scheduling app that helps teachers submit workday outlines
MIT License
5 stars 5 forks source link

Big improvements to the `./manage.py seed`-script #112

Closed Greenheart closed 8 years ago

Greenheart commented 8 years ago

The script now inserts:


Updates:

Note:

Greenheart commented 8 years ago

@Limpan Should I do a db-migration for this PR as well?

Limpan commented 8 years ago

Have you changed models.py? If yes, you probably need to, otherwise you shouldn't.

Greenheart commented 8 years ago

Then I'll update this PR in about an hour

@Limpan Edit: This didn't require a migration since my changes didn't affect the schema. (View output by clicking ... below)

This is ready to be QA:ed and merged :shipit:

(venv) sam@ubbe:~/Documents/github/chronos/web$ ./manage.py db migrate -m "Added init-methods to the models" Importing environment from .env... INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.ddl.postgresql] Detected sequence named 'users_id_seq' as owned by integer column 'users(id)', assuming SERIAL and omitting INFO [alembic.ddl.postgresql] Detected sequence named 'work_periods_id_seq' as owned by integer column 'work_periods(id)', assuming SERIAL and omitting INFO [alembic.ddl.postgresql] Detected sequence named 'workdays_id_seq' as owned by integer column 'workdays(id)', assuming SERIAL and omitting INFO [alembic.ddl.postgresql] Detected sequence named 'deviations_id_seq' as owned by integer column 'deviations(id)', assuming SERIAL and omitting INFO [alembic.env] No changes in schema detected.

Limpan commented 8 years ago

Why the __init__-methods at all. You don't seem to use them anyway, or am I missing something?

Greenheart commented 8 years ago

I do use the __init__-methods to pass in some non-default data. (See the db.session.add()-calls in manage,.py)

My thought is that they will be useful when we in the future want to use customized objects. For example these methods would provide the flexibility required to automate tests for approving/rejecting schedules or just want to test that the front-end properly hides inactive users without requiring us to manually "delete" them first.

Limpan commented 8 years ago

http://docs.sqlalchemy.org/en/rel_1_0/orm/tutorial.html#working-with-related-objects

Greenheart commented 8 years ago

@Limpan Oh, I didn't know that! :santa:

What should I do to keep the pre-defined __init__-method but still be able to override some of the default parameters?

  1. By just removing my __init__-methods and creating new objects like nothing else changed?
  2. Some other way?
Limpan commented 8 years ago

I'm not blaming you, just informing. 😄

Remove the __init__-methods and change to named arguments since there's very few locations.

Greenheart commented 8 years ago

@Limpan And I took it as a chance to learn, not blame. When :santa: is around, you know it's serious stuff! :smile:

And the named arguments should match the local variables (field_names) assigned inside each model-class?

Limpan commented 8 years ago

I thought you would.

That is correct. The default __init__-method allows, as named arguments, all of the attributes declared in the class. Pretty and practical.

Greenheart commented 8 years ago

@Limpan Nice! I'll update the PR in a moment

Greenheart commented 8 years ago

Models no longer override their inherited __init__-methods

Also fixed a bug where all Deviations got added to the same schedule because of a db-query that found the first schedule in a given WorkPeriod instead of the first schedule in the WP for the current user.