Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Admin extensions #185

Merged
merged 31 commits into from
Aug 14, 2015
Merged

Admin extensions #185

merged 31 commits into from
Aug 14, 2015

Conversation

mjumbewu
Copy link
Member

This should close issue #162 (organizational changes through the admin) and #173 (export affordance).

Some things to be aware of:

  • The schema changed. There is a more explicit relationship between users and departments. Previously, users had a string field representing any department they were primary or backup contact for. Now that relationship is represented by primary key fields on departments to users.
  • Adding relationships between users and departments in the admin requires that both be created first.
  • The "export affordance" is really simple; a link.

The admin page currently uses the fields that are used in the `STAFF_URL` CSV
file: Name (alias), Email, Department, and Phone Number (phone). Also, the
`is_staff` field is exposed so that administrators can see all users (like the
non-staff liasons).
Allow unrestricted access to the admin views
In a recent commit, I added the ability to administer system users through the
web interface. This commit allows the same for departments.

* Add `primary_contact_id` and `backup_contact_id` fields to the `Department`
  model, so that the Flask admin extension can reason about the relationship.
* Change the `department_id` field to use `use_alter` flag, so that the user
  table and the department table can both be created without a circular
  dependency error.

This commit does introduce changes that make upgrades difficult; the effort in
creating migrations isn't worth it.
When a user first goes to create a department, they have empty lists for both
primary and backup contact options. We want the user to know that the contacts
correspond to system users.
I changed the foreign key on a `User` referring to a `Department` from
*department* to *department_id*. This commit updates various references to the
field.
* Update the version of Flask-Admin from 1.0.6 to 1.0.9. The latest provides
  more options for overriding template blocks.
* Add an export link in to the menu_links template block.
* Add a "Content-disposition" header that designates the file as a
  downloadable attachment.
* Add a default file name ("records.csv"), and take an optional query string
  argument (`filename`) to set a different file name.
@waltz
Copy link
Member

waltz commented Apr 21, 2015

I'm not super active on this project and I'm not in a place to merge this, but I want to give a huge thumbs up 👍 to active work on this project.

The string representation just refers to the underlying *user* instance. This
may introduce another query for each displayed owner and subscriber, unless
the *user* instances are pre-joined.
Always join the `user` attribute when loading owners and subscribers to
avoid the situation described in commit #52c370b
It is a legal requirement that record request receipts not be post-dated. This
change enforces that within the Record admin pages
- Add javascript validators for the form
- Add simple value validation on the server, just in case something gets past
  the javascript validation

This commit along with commit fed1b92 should close issue #178.
Used to return `False` no matter what. Now returns `True` in the appropriate
cases.
@richaagarwal
Copy link
Member

@mjumbewu Thanks for this work! I pulled the branch down locally and noticed that the /view_requests page doesn't load (in addition to 8 test failures) -- is this still a WIP?

@mjumbewu
Copy link
Member Author

mjumbewu commented May 5, 2015

@richaagarwal The /view_requests route loads fine for me. What does it look like to you? 404? 500? Blank screen?

And yeah, I'm still looking into those failing tests.

@richaagarwal
Copy link
Member

@mjumbewu Woops, had to start with a fresh database, my bad -- /view_requests is all good.

@@ -31,14 +31,17 @@ class User(db.Model):
phone = db.Column(db.String())
date_created = db.Column(db.DateTime)
password = db.Column(db.String(255))
department = db.Column(Integer, ForeignKey("department.id"))
current_department = relationship("Department", foreign_keys = [department], uselist = False)
department_id = db.Column(Integer, ForeignKey("department.id", use_alter=True, name="fk_department"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to do this renaming? If so, a migration would be helpful.

The Flask login extension allows disabling login with either the
`LOGIN_DISABLED` or the `TESTING` variable. Loading the former as
we were doing conflicted with `login_required`'s default behavior.
Also changed the *alembic/env.py* file to use the `SQLALCHEMY_DATABASE_URI`
from the app config, as that's where the `DATABASE_URL` from the environment
is stored.

    foreman run alembic upgrade head
Addresses issue #176, "force responder to select a reason when submitting a letter of extension".
This, along with commits 7a9b0ac and 9c420e7 should at least partially resolve
issue #172 as I undersand it. I think the problem is that the requested dates
weren't being displayed, even though they were stored in the database.

There is another side to the story: The due_date may still report a premature
overdue status, because it's set relative to the date_created. The set_due_date
method is going to have to become smarter to take into account the time of day
that the request was made. For now, displaying the time at least allows users
to spot check whether the request is ACTUALLY overdue.
richaagarwal added a commit that referenced this pull request Aug 14, 2015
@richaagarwal richaagarwal merged commit 03958e0 into master Aug 14, 2015
@richaagarwal richaagarwal deleted the admin-extensions branch September 16, 2015 06:52
joelbcastillo referenced this pull request in CityOfNewYork/NYCOpenRecords Mar 2, 2017
Bugfix/OP-1063: Fix to Unformatted Date in Add File Email
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants