-
Notifications
You must be signed in to change notification settings - Fork 12
Add a Parking Lot #53
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
# Generated by Django 4.2.19 on 2025-03-07 23:55 | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
dependencies = [ | ||
("commitfest", "0010_add_failing_since_column"), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name="patchoncommitfest", | ||
name="status", | ||
field=models.IntegerField( | ||
choices=[ | ||
(1, "Needs review"), | ||
(2, "Waiting on Author"), | ||
(3, "Ready for Committer"), | ||
(4, "Committed"), | ||
(5, "Moved to next CF"), | ||
(6, "Rejected"), | ||
(7, "Returned with feedback"), | ||
(8, "Withdrawn"), | ||
(9, "Moved to Parking Lot"), | ||
], | ||
default=1, | ||
), | ||
), | ||
migrations.RunSQL(""" | ||
INSERT INTO commitfest_patchstatus (status, statusstring, sortkey) VALUES | ||
(1,'Needs review',10), | ||
(2,'Waiting on Author',15), | ||
(3,'Ready for Committer',20), | ||
(4,'Committed',25), | ||
(5,'Moved to next CF',30), | ||
(6,'Rejected',50), | ||
(7,'Returned with Feedback',50), | ||
(8,'Withdrawn', 50), | ||
(9,'Moved to Parking Lot', 30) | ||
ON CONFLICT (status) DO UPDATE SET statusstring=excluded.statusstring, sortkey=excluded.sortkey; | ||
"""), | ||
migrations.RunSQL( | ||
"DELETE FROM commitfest_patchstatus WHERE status < 1 OR status > 9" | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
# Generated by Django 4.2.19 on 2025-03-08 03:56 | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
dependencies = [ | ||
("commitfest", "0011_patch_status_parked"), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name="commitfest", | ||
name="status", | ||
field=models.IntegerField( | ||
choices=[ | ||
(1, "Future"), | ||
(2, "Open"), | ||
(3, "In Progress"), | ||
(4, "Closed"), | ||
(5, "Parking"), | ||
], | ||
default=1, | ||
), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,17 +38,20 @@ class CommitFest(models.Model): | |
STATUS_OPEN = 2 | ||
STATUS_INPROGRESS = 3 | ||
STATUS_CLOSED = 4 | ||
STATUS_PARKING = 5 | ||
_STATUS_CHOICES = ( | ||
(STATUS_FUTURE, "Future"), | ||
(STATUS_OPEN, "Open"), | ||
(STATUS_INPROGRESS, "In Progress"), | ||
(STATUS_CLOSED, "Closed"), | ||
(STATUS_PARKING, "Parking"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Seems good to change the name to "Parking Lot" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as discussed in the other thread. Let's call it "Drafts" |
||
) | ||
_STATUS_LABELS = ( | ||
(STATUS_FUTURE, "default"), | ||
(STATUS_OPEN, "info"), | ||
(STATUS_INPROGRESS, "success"), | ||
(STATUS_CLOSED, "danger"), | ||
(STATUS_PARKING, "info"), | ||
) | ||
name = models.CharField(max_length=100, blank=False, null=False, unique=True) | ||
status = models.IntegerField( | ||
|
@@ -228,6 +231,7 @@ class PatchOnCommitFest(models.Model): | |
STATUS_REJECTED = 6 | ||
STATUS_RETURNED = 7 | ||
STATUS_WITHDRAWN = 8 | ||
STATUS_PARKED = 9 | ||
_STATUS_CHOICES = ( | ||
(STATUS_REVIEW, "Needs review"), | ||
(STATUS_AUTHOR, "Waiting on Author"), | ||
|
@@ -237,6 +241,7 @@ class PatchOnCommitFest(models.Model): | |
(STATUS_REJECTED, "Rejected"), | ||
(STATUS_RETURNED, "Returned with feedback"), | ||
(STATUS_WITHDRAWN, "Withdrawn"), | ||
(STATUS_PARKED, "Moved to Parking Lot"), | ||
) | ||
_STATUS_LABELS = ( | ||
(STATUS_REVIEW, "default"), | ||
|
@@ -247,6 +252,7 @@ class PatchOnCommitFest(models.Model): | |
(STATUS_REJECTED, "danger"), | ||
(STATUS_RETURNED, "danger"), | ||
(STATUS_WITHDRAWN, "danger"), | ||
(STATUS_PARKED, "warning"), | ||
) | ||
OPEN_STATUSES = [STATUS_REVIEW, STATUS_AUTHOR, STATUS_COMMITTER] | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,7 +141,7 @@ def activity(request, cfid=None, rss=None): | |
cf = None | ||
where = "" | ||
|
||
sql = "SELECT ph.date, auth_user.username AS by, ph.what, p.id AS patchid, p.name, (SELECT max(commitfest_id) FROM commitfest_patchoncommitfest poc WHERE poc.patch_id=p.id) AS cfid FROM commitfest_patchhistory ph INNER JOIN commitfest_patch p ON ph.patch_id=p.id INNER JOIN auth_user on auth_user.id=ph.by_id {0} ORDER BY ph.date DESC LIMIT {1}".format( | ||
sql = "SELECT ph.date, auth_user.username AS by, ph.what, p.id AS patchid, p.name FROM commitfest_patchhistory ph INNER JOIN commitfest_patch p ON ph.patch_id=p.id INNER JOIN auth_user on auth_user.id=ph.by_id {0} ORDER BY ph.date DESC LIMIT {1}".format( | ||
where, num | ||
) | ||
|
||
|
@@ -323,7 +323,7 @@ def patchlist(request, cf, personalized=False): | |
EXISTS ( | ||
SELECT 1 FROM commitfest_patch_authors cpa WHERE cpa.patch_id=p.id AND cpa.user_id=%(self)s | ||
) AND ( | ||
poc.commitfest_id < %(cid)s | ||
cf.status = %(status_closed)s | ||
) | ||
THEN 'Your still open patches in a closed commitfest (you should move or close these)' | ||
WHEN | ||
|
@@ -349,6 +349,7 @@ def patchlist(request, cf, personalized=False): | |
cf.name AS cf_name, | ||
cf.status AS cf_status, | ||
""" | ||
whereparams["status_closed"] = CommitFest.STATUS_CLOSED | ||
whereparams["needs_author"] = PatchOnCommitFest.STATUS_AUTHOR | ||
whereparams["needs_committer"] = PatchOnCommitFest.STATUS_COMMITTER | ||
is_committer = bool(Committer.objects.filter(user=request.user, active=True)) | ||
|
@@ -515,7 +516,13 @@ def commitfest(request, cfid): | |
# Generate patch status summary. | ||
curs = connection.cursor() | ||
curs.execute( | ||
"SELECT ps.status, ps.statusstring, count(*) FROM commitfest_patchoncommitfest poc INNER JOIN commitfest_patchstatus ps ON ps.status=poc.status WHERE commitfest_id=%(id)s GROUP BY ps.status ORDER BY ps.sortkey", | ||
"""SELECT ps.status, ps.statusstring, count(*) | ||
FROM commitfest_patchoncommitfest poc | ||
INNER JOIN commitfest_patchstatus ps ON ps.status=poc.status | ||
INNER JOIN commitfest_commitfest cf ON cf.id=poc.commitfest_id | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm probably missing something obvious, but why was this join added? The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's used in |
||
WHERE commitfest_id=%(id)s | ||
GROUP BY ps.status | ||
ORDER BY ps.sortkey""", | ||
{ | ||
"id": cf.id, | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems incorrect? I.e. it won't remove anything afaict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I meant to add an explanation to the commit but didn't. This is fully cargo-culted from the last change made to
commitfest_patchstatus
: 535af6eIf this table doesn't need that much pruning in practice, that's fine and I can remove it. But I can't take a look at the current state of the prod database to really know for sure 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Maybe @mhagander knows why this should or should not be done this time around?)