Merge lp:~sinzui/launchpad/sprint-is-physical into lp:launchpad/db-devel

This is my branch to add a is_physical flag for SprintAttendance. This
branch just adds the column tot he DB so that edge users to test this
feature as it is developed in October.

    lp:~sinzui/launchpad/sprint-in-person
    Diff size: 496 (thank you make newsampledata)
    Launchpad bug: https://bugs.launchpad.net/bugs/430945
    Test command: make schema
    Pre-implementation: no one
    Target release: 3.1.0

= Add a is_physical flag for SprintAttendance =

In order to better plan UDS we'd like the sprint/meeting feature to have a
checkbox so we can tell when an attendee is physically attending an event or
participating remotely. Something like:

    [ ] I plan to physically attend this event at $location.

Curtis adds "The background of this issue reminds me a request to modify the
person profile to have a t-shirt size field for sprints."

ADDENDUM 1

There is no bug for the t-shirt, but I have been asked it often enough from
HR that I offer to add this on my own time. If you feel the column is not
worthy of Launchpad, my feelings will not be hurt.

ADDENDUM 2

The DB has SprintAttendance.date_created, but the interface and model
do not know about this data. I suspect there was a place to capture this
information. I imagine the migration script would use the start_date of
the sprint as the date created for the attendance. This is not important
to me so I have chosen to settle this matter later.

== Rules ==

    * Add is_physical to SprintAttendance
      The value is BOOLEAN
    * t_shirt_size text to Person.
      The interface will define an enum:
      UNKNOWN, SMALL, MEDIUM, LARGE, EXTRA_LARGE, EXTRA_EXTRA_LARGE

== Lint ==

Linting changed files:
  database/sampledata/current-dev.sql
  database/sampledata/current.sql
  database/schema/comments.sql
  database/schema/patch-2109-97-0.sql

== Implementation ==

    * database/sampledata/current-dev.sql
      * Updated using make newsampledata.
    * database/sampledata/current.sql
      * Updated using make newsampledata.
    * database/schema/comments.sql
      * Added comments for the two columns I am adding.
      * Added missing comments for the sprint and attendee
    * database/schema/patch-2109-97-0.sql
      * Add the SprintAttendance.is_physical and Person.t_shirt_size columns.

It is incorrect to have t-shirt size on person, as sizes change depending on location. For example, if I'm registered for sprints in the USA and Asia I'll need a Large for the US sprint and a X-Large for the Asian one. Also, You most likely need a flag on the Sprint table, specifying if t-shirts may be offered - no point asking people for their t-shirt sizes if there will be none.

> ADDENDUM 2
>
> The DB has SprintAttendance.date_created, but the interface and model
> do not know about this data. I suspect there was a place to capture this
> information. I imagine the migration script would use the start_date of
> the sprint as the date created for the attendance. This is not important
> to me so I have chosen to settle this matter later.

SprintAttendance.date_created seems fine - it is the creation date of the record in the database, similar to many other date_created fields. I don't see any particular need for the Launchpad application itself to know or care about this information; we just capture it in case we need it and maybe to draw pretty graphs from.

I agree with stub regarding Person.t_shirt_size. Perhaps SprintAttendance would be the least bad place for this. The form code could then use the person's last given t-shirt size as a best-guess default.

It makes me wonder though, what else do sprint organizers need to know? I can't see any field in the database for storing a person's dietary requirements, for example. What are we actually going to do with this data?

The 'is_physical' flag makes sense. 'date_created' might have some actual uses (e.g. a prize to the first person who signs up), but I don't think that it's worth worrying about right now.

Hi Jono and Stuart.

Thanks for the review. Stuarts argument against the t-shirt is very
convincing. I dropped the column. I still hope to land this for 3.0 so
that UDS planning can use the feature on edge.

On Fri, 2009-09-18 at 05:19 +0000, Jonathan Lange wrote:
> Review: Needs Fixing db
> I agree with stub regarding Person.t_shirt_size. Perhaps
> SprintAttendance would be the least bad place for this. The form code
> could then use the person's last given t-shirt size as a best-guess
> default.
>
> It makes me wonder though, what else do sprint organizers need to
> know? I can't see any field in the database for storing a person's
> dietary requirements, for example. What are we actually going to do
> with this data?
>
> The 'is_physical' flag makes sense. 'date_created' might have some
> actual uses (e.g. a prize to the first person who signs up), but I
> don't think that it's worth worrying about right now.

This is the complete patch file:

{{{

-- Copyright 2009 Canonical Ltd. This software is licensed under the
-- GNU Affero General Public License version 3 (see the file LICENSE).

SET client_min_messages=ERROR;

-- Add a column to indicate that the attendee is physically attending
-- the sprint.
ALTER TABLE SprintAttendance
  ADD COLUMN is_physical BOOLEAN NOT NULL DEFAULT FALSE;

INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 97, 0);

}}}

This is the incremental diff sans sampledata changes.

{{{

=== modified file 'database/schema/comments.sql'
--- a/database/schema/comments.sql 2009-09-18 03:18:28 +0000
+++ b/database/schema/comments.sql 2009-09-18 14:28:55 +0000
@@ -1150,7 +1150,6 @@
 COMMENT ON COLUMN Person.mailing_list_receive_duplicates IS 'True means the user wants to receive list copies of messages on which they are explicitly named as a recipient.';
 COMMENT ON COLUMN Person.visibility IS 'person.PersonVisibility enumeration which can be set to Public, Public with Private Membership, or Private.';
 COMMENT ON COLUMN Person.verbose_bugnotifications IS 'If true, all bugnotifications sent to this Person will include the bug description.';
-COMMENT ON COLUMN Person.t_shirt_size IS 'The t-shirt size the person prefers when attending a sprint.';

 COMMENT ON VIEW ValidPersonCache IS 'A materialized view listing the Person.ids of all valid people (but not teams).';

=== renamed file 'database/schema/patch-2109-97-0.sql' => 'database/schema/patch-2207-97-0.sql'
--- a/database/schema/patch-2109-97-0.sql 2009-09-18 02:27:28 +0000
+++ b/database/schema/patch-2207-97-0.sql 2009-09-18 14:28:35 +0000
@@ -10,10 +10,4 @@
   ADD COLUMN is_physical BOOLEAN NOT NULL DEFAULT FALSE;

--- Add a column to to specify the user's t-shirt-size for sprints.
--- 0 UNKNOWN, 1 SMALL, 2 MEDIUM, 3 LARGE, 4 EXTRA_LARGE, 5 EXTRA_EXTRA_LARGE
-ALTER TABLE Person
- ADD COLUMN t_shirt_size INT NOT NULL DEFAULT 0;
-
-
-INSERT INTO LaunchpadDatabaseRevision VALUES (2109, 97, 0);
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 97, 0);

}}}

I'm OK with this patch, pending stub's approval.

Approved as patch-2207-04-0.sql

I'm not that hot on the name of the new column, but it is good enough for now.

A db patch cannot be included in a re-roll unless it is critical to the on-going operation of Launchpad.

We are not including DB patches in the re-roll.

If this is really critical to UDS success, we can ask Stuart to manually apply the DB patch.

> If this is really critical to UDS success, we can ask Stuart to manually apply
> the DB patch.

I can't add new columns without downtime and/or switching on read-only mode due to replication issues.