Launchpad itself

Merge lp:~bac/launchpad/bug-490505 into lp:launchpad/db-devel

  1. bug-490505
  2. Merge into db-devel
Proposed by Brad Crittenden
Status: Merged
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-490505
Merge into: lp:launchpad/db-devel
Diff against target: 17 lines (+13/-0)
1 file modified
database/schema/patch-2207-20-0.sql (+13/-0)
To merge this branch: bzr merge lp:~bac/launchpad/bug-490505
Reviewer Review Type Date Requested Status
Данило Шеган (community) release-critical Disapprove
Jonathan Lange (community) db Approve
Stuart Bishop (community) db Approve
Review via email: mp+15691@code.launchpad.net

Commit message

Change the ValidPersonOrTeamCache db view to not included merged teams.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

ValidPersonOrTeamCache was including teams that had been merged and really should not
be considered valid. This patch removes them from the view.

== Proposed fix ==

db patch to redefine the view to not include merged teams.

== Pre-implementation notes ==

None.

== Implementation details ==

As above.

== Tests ==

None.

== Demo and Q/A ==

Demo by merging a team in the UI and then running a query on the view to ensure the
merged team is not there.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  database/schema/patch-2207-99-0.sql

Revision history for this message
Stuart Bishop (stub) wrote :

Good catch - we didn't have team merging when this view was last modified.

Approved as patch-2207-20-0.sql

review: Approve (db)
Revision history for this message
Jonathan Lange (jml) wrote :

Fine by me.

review: Approve (db)
Revision history for this message
Данило Шеган (danilo) wrote :

There are several concerns:
 * the bug is already targetted at 3.1.13
 * we don't know how this affects DB upgrade timing
 * this doesn't seem that urgent considering how long it's been in the loop
 * and, it's a DB patch which makes stuff much harder for everyone, and would kick off the staging DB restore, thus not having staging pick up code updates every half an hour
 * even if staging restore is done, there's not enough time to QA this

As such, I'd rather see this moved to the next cycle, unless there's a particularly strong reason (i.e. this is critical) to do it now.

I am disapproving for now, but I am open to be convinced otherwise.

review: Disapprove (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'database/schema/patch-2207-20-0.sql'
2--- database/schema/patch-2207-20-0.sql 1970-01-01 00:00:00 +0000
3+++ database/schema/patch-2207-20-0.sql 2009-12-18 22:00:34 +0000
4@@ -0,0 +1,13 @@
5+SET client_min_messages=ERROR;
6+
7+-- Drop the old view.
8+DROP VIEW validpersonorteamcache;
9+
10+-- Create the new view that excludes merged teams.
11+CREATE VIEW validpersonorteamcache AS
12+ SELECT person.id FROM
13+ ((person LEFT JOIN emailaddress ON ((person.id = emailaddress.person))) LEFT JOIN account ON ((emailaddress.account = account.id)))
14+ WHERE (((person.teamowner IS NOT NULL) AND (person.merged IS NULL)) OR
15+ (person.teamowner IS NULL AND (account.status = 20) AND (emailaddress.status = 4)));
16+
17+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 20, 0);

Subscribers

People subscribed via source and target branches

to status/vote changes: