1097967 - Add Region Table to DB

Status: RESOLVED FIXED

(Webtools Graveyard :: Air Mozilla, defect, P2)

Description

[Richard A Milewski[:richard]]

As an aid to improving metrics, we should add a region table to the database.

Locations can be assigned to a region (just as they have a time zone).

That would let us aggregate event counts for regions automagically.

Comment 1

[Peter Bengtsson [:peterbe]]

Do we have a known list of regions? If so, we could hardcode the options for a drop-down widget.

Comment 2

[Peter Bengtsson [:peterbe]]

We talked about this, we want Region to be a new database model thing. And the Location should have a many-to-many relationship to regions (it can start as nothing).

We need a way to add, browse, edit these regions in the management. Just like we do today with things like Users, Video Templates, etc. 

So, the Location model will then have:

* ID (implied)
* Name
* Timezone
* Regions

Comment 3

[Peter Bengtsson [:peterbe]]

Lisa, this one is a bit more substantial but shouldn't be too hard since it's not editing anything. Just adding new functionality. 

We can build much of this simply by "copy-n-pasting" from some existing functionality.

Comment 4

[Mike Hoye [:mhoye]]

If we can get a DXR/source link here, I can start shopping this around as a good first bug. Thanks.

Comment 5

[Peter Bengtsson [:peterbe]]

(In reply to Mike Hoye [:mhoye] from comment #4)
> If we can get a DXR/source link here, I can start shopping this around as a
> good first bug. Thanks.

DXR?

To start "shopping this around" you can follow the https://github.com/mozilla/airmozilla/blob/master/README.md to get started working on this.

Comment 6

[Mike Hoye [:mhoye]]

Hi, Peter - Thanks for that, but I was hoping that you could provide a link to the file where this change would need to be made and a rough description of what the fix would look like, so that I could start advertising this bug in our various engagement pathways.

Comment 7

[Peter Bengtsson [:peterbe]]

(In reply to Mike Hoye [:mhoye] from comment #6)
> Hi, Peter - Thanks for that, but I was hoping that you could provide a link
> to the file where this change would need to be made and a rough description
> of what the fix would look like, so that I could start advertising this bug
> in our various engagement pathways.

There are multiple files. ...that need to be edited. We also have lots of new files that need to be created. And you need to write migration scripts. It's all standard Django stuff and I can guide but it's a bit more involved than just editing some files. 
I think the rough description is comment #3 and usually with these things, if someone picks this up they'll communicate with us on #airmozilla-dev or GitHub. 

In Q1 next year I intend to set aside some decent chunks of time to make all the new-developer-onramping more welcoming.

Comment 8

[Mike Hoye [:mhoye]]

OK, if that's the case this probably doesn't make for a good first bug; thanks for clarifying.

Comment 9

[Lisa Hewus Fresh [:lfresh]]

May I get a little more clarification about this? How do you want this to function in the dashboard and what are the basic steps for implementation?

Comment 10

[Peter Bengtsson [:peterbe]]

Lisa, 

I think the best approach is to think of this as a clone of the Locations functionality. We'll need the ability to add one, edit one and remove one. (Note, we might not allow removal in terms of actual deletion but use a "Mark as removed" functionality instead). 

If you poke around the files to do with Locations, that's generally what you'll be "copying". E.g. locations.html, locations.js, location_edit.html and the equivalent relevant functions in manage/views.py

What this bug is all about is to tie the regions to locations later. I would worry about that last. So, when you've built the functionality and added a "Region" model in main/models.py then you can make a migration for creating that. Once that's settled, you're going to need to make a new migration just for adding Regions to Locations and that'll involved changing location_edit.html and location_new.html etc. 

I think what you need is something really simple for a class. I would think something like this should be enough::

  class Region(models.Model):
    name = models.CharField(max_length=300)
    is_active = models.BooleanField(default=True)

    class Meta:
        ordering = ['name']

    def __unicode__(self):
        return self.name

In terms for deletion, I think you should make it so that the user can click a "Remove" button but you be smart in the code for that. If the Region to be deleted is not used by ANY Location, then genuinely delete it from the database but if it IS used, instead set it to is_active=False so it stops appearing as an option in the Location edit. 

If in doubt, find us on this bug or on IRC.

Comment 11

[Lisa Hewus Fresh [:lfresh]]

https://github.com/mozilla/airmozilla/pull/180

Comment 12

[[github robot]]

Commits pushed to master at https://github.com/mozilla/airmozilla

https://github.com/mozilla/airmozilla/commit/28282ac9e43507834c9ceba28f91e7568d8d1b68
Bug 1097967 - Updates and migrations for linking regions to locations

Replacing with a fresh copy to fix runserver errors

Bug 1097967 - Updates for review - Add Regions Table

Bug 1097967 - Add migration for Add Regions Table

Bug 1097967 - Edits to region_edit.html

Bug 1097967 - Updates for Add Region Table to DB

Removing deprecated vendor-local directory

small fix

Bug 1097967 - Updates and fixes for previous pull request.

Merge pull request #1 from peterbe/1097967_peter

small fix
Bug 1097967 - Fixes for models.py

Bug 1097967 - Fixed Locations model

Bug 1097967 - Added tests for Add Regions Table to DB

Bug 1097967 - Updates/fixes to tests for Add Regions Table to DB

https://github.com/mozilla/airmozilla/commit/a97017d0522ac26d04e1bdafba3fa3e3db0cf52b
Merge pull request #180 from bugzPDX/1097967

Bug 1097967 - Add Regions Table to DB

Comment 13

[Peter Bengtsson [:peterbe]]

Thank you Lisa!