Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

WIP: Try out botstrap5#3

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
JelteF wants to merge12 commits intomain
base:main
Choose a base branch
Loading
frombootstrap5
Draft
Show file tree
Hide file tree
Changes from1 commit
Commits
Show all changes
12 commits
Select commitHold shift + click to select a range
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
PrevPrevious commit
NextNext commit
Make /patch/{id} the URL for a patch
Previously we'd include the ID of the commitfest in the URL of thepatch. In9f12a5e we introduced a stable URL for patches that wouldredirect to the one for the latest commitfest. This starts to use thatURL as the valid only URL for a patch (with the previous URL redirectingto this one).The reasoning behind this is that the old approach resulted in Ndifferent URLs for each patch, which all showed the exact same patchinformation. The only difference between all these URLs would be thebreadcrumb at the top of the page.The only benefit of that approach is that if you're on an oldcommitfest, and click a link there, then the breadcrumb will bring youback to where you came from. Since people rarely have a reason to browseclosed commitfests, the that benefit seems pretty small. Especiallybecause people can just as well press their browser back button, in thatcase.The problems that these N links cause seem much more impactful to mostusers:1. If you click an old link to a cf entry (e.g. one in the email   archives), then the breadcrumb will contain some arbitrarily old   commitfest. It seems much more useful to have the breadcrumb show the   commitfest that the patch is currently active in (or got   committed/rejected in).2. Places that use the stable URLs require an extra round-trip to   actually get to the patch page.3. It's a bit confusing that old pages of a patch still get updated with   all the new information, i.e. why have all these pages if they   contain the exact same content.4. Problem 3 is generally also bad for Search Engine Optimization (SEO),   for now we don't care much about that though.Finally this also changes the links on the patch page itself for each ofthe commitfests that a patch has been part of. Those links were alreadyrather useless, since all they effectively did was change thebreadcrumb. But with this new commit, they wouldn't even do that anymore,and simply redirect to the current page. So now they start pointing tothe commitfest itself, which seems more useful behaviour anyway.
  • Loading branch information
@JelteF
JelteF committedJan 13, 2025
commitbf5f948d61579b2e6b51b96e88cbcbc79a49f0b0
3 changes: 3 additions & 0 deletionspgcommitfest/commitfest/models.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -122,6 +122,9 @@ class Patch(models.Model, DiffableModel):
'reviewers': 'reviewers_string',
}

def current_commitfest(self):
return self.commitfests.order_by('-startdate').first()

# Some accessors
@property
def authors_string(self):
Expand Down
2 changes: 1 addition & 1 deletionpgcommitfest/commitfest/templates/commitfest.html
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -85,7 +85,7 @@ <h3>{{p.is_open|yesno:"Active patches,Closed patches"}}</h3>
{%endifchanged%}
{%endif%}
<tr>
<td><a href="{{p.id}}/">{{p.name}}</a></td>
<td><a href="/patch/{{p.id}}/">{{p.name}}</a></td>
<td>{{p.id}}</td>
<td><span class="label label-{{p.status|patchstatuslabel}}">{{p.status|patchstatusstring}}</span></td>
<td>{%if p.targetversion%}<span class="label label-default">{{p.targetversion}}</span>{%endif%}</td>
Expand Down
2 changes: 1 addition & 1 deletionpgcommitfest/commitfest/templates/patch.html
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -68,7 +68,7 @@
<tr>
<th>Status</th>
<td>{%for c in patch_commitfests %}
<div style="margin-bottom: 3px;"><a href="/{{c.commitfest.id}}/{{patch.id}}/">{{c.commitfest}}</a>: <span class="label label-{{c.status|patchstatuslabel}}">{{c.statusstring}}</span></div>
<div style="margin-bottom: 3px;"><a href="/{{c.commitfest.id}}/">{{c.commitfest}}</a>: <span class="label label-{{c.status|patchstatuslabel}}">{{c.statusstring}}</span></div>
{%endfor%}
</td>
</tr>
Expand Down
54 changes: 31 additions & 23 deletionspgcommitfest/commitfest/views.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,17 +320,18 @@ def global_search(request):
})


def patch_redirect(request, patchid):
last_commitfest = PatchOnCommitFest.objects.select_related('commitfest').filter(patch_id=patchid).order_by('-commitfest__startdate').first()
if not last_commitfest:
raise Http404("Patch not found")
return HttpResponseRedirect(f'/{last_commitfest.commitfest_id}/{patchid}/')
def patch_legacy_redirect(request, cfid, patchid):
# Previously we would include the commitfest id in the URL. This is no
# longer the case.
return HttpResponseRedirect(f'/patch/{patchid}/')


def patch(request, cfid, patchid):
cf = get_object_or_404(CommitFest, pk=cfid)
patch = get_object_or_404(Patch.objects.select_related(), pk=patchid, commitfests=cf)
patch_commitfests = PatchOnCommitFest.objects.select_related('commitfest').filter(patch=patch).order_by('-commitfest__startdate')
def patch(request, patchid):
patch = get_object_or_404(Patch.objects.select_related(), pk=patchid)
cf = patch.current_commitfest()

patch_commitfests = PatchOnCommitFest.objects.select_related('commitfest').filter(patch=patch).order_by('-commitfest__startdate').all()

committers = Committer.objects.filter(active=True).order_by('user__last_name', 'user__first_name')

cfbot_branch = getattr(patch, 'cfbot_branch', None)
Expand DownExpand Up@@ -373,9 +374,9 @@ def patch(request, cfid, patchid):

@login_required
@transaction.atomic
def patchform(request,cfid,patchid):
cf = get_object_or_404(CommitFest, pk=cfid)
patch =get_object_or_404(Patch, pk=patchid, commitfests=cf)
def patchform(request, patchid):
patch = get_object_or_404(Patch, pk=patchid)
cf =patch.current_commitfest()

prevreviewers = list(patch.reviewers.all())
prevauthors = list(patch.authors.all())
Expand DownExpand Up@@ -465,9 +466,9 @@ def _review_status_string(reviewstatus):

@login_required
@transaction.atomic
def comment(request, cfid, patchid, what):
cf = get_object_or_404(CommitFest, pk=cfid)
def comment(request, patchid, what):
patch = get_object_or_404(Patch, pk=patchid)
cf = patch.current_commitfest()
poc = get_object_or_404(PatchOnCommitFest, patch=patch, commitfest=cf)
is_review = (what == 'review')

Expand DownExpand Up@@ -562,8 +563,10 @@ def comment(request, cfid, patchid, what):

@login_required
@transaction.atomic
def status(request, cfid, patchid, status):
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cfid, patch__id=patchid)
def status(request, patchid, status):
patch = get_object_or_404(Patch.objects.select_related(), pk=patchid)
cf = patch.current_commitfest()
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cf.id, patch__id=patchid)

if poc.is_closed:
# We allow modification of patches in closed CFs *only* if it's the
Expand DownExpand Up@@ -597,8 +600,10 @@ def status(request, cfid, patchid, status):

@login_required
@transaction.atomic
def close(request, cfid, patchid, status):
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cfid, patch__id=patchid)
def close(request, patchid, status):
patch = get_object_or_404(Patch.objects.select_related(), pk=patchid)
cf = patch.current_commitfest()
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cf.id, patch__id=patchid)

if poc.is_closed:
# We allow modification of patches in closed CFs *only* if it's the
Expand DownExpand Up@@ -695,8 +700,7 @@ def close(request, cfid, patchid, status):

@login_required
@transaction.atomic
def reviewer(request, cfid, patchid, status):
get_object_or_404(CommitFest, pk=cfid)
def reviewer(request, patchid, status):
patch = get_object_or_404(Patch, pk=patchid)

is_reviewer = request.user in patch.reviewers.all()
Expand All@@ -715,7 +719,6 @@ def reviewer(request, cfid, patchid, status):
@login_required
@transaction.atomic
def committer(request, cfid, patchid, status):
get_object_or_404(CommitFest, pk=cfid)
patch = get_object_or_404(Patch, pk=patchid)

committer = list(Committer.objects.filter(user=request.user, active=True))
Expand All@@ -740,8 +743,7 @@ def committer(request, cfid, patchid, status):

@login_required
@transaction.atomic
def subscribe(request, cfid, patchid, sub):
get_object_or_404(CommitFest, pk=cfid)
def subscribe(request, patchid, sub):
patch = get_object_or_404(Patch, pk=patchid)

if sub == 'un':
Expand All@@ -754,6 +756,12 @@ def subscribe(request, cfid, patchid, sub):
return HttpResponseRedirect("../")


def send_patch_email(request, patchid):
patch = get_object_or_404(Patch, pk=patchid)
cf = patch.current_commitfest()
return send_email(request, cf.id)


@login_required
@transaction.atomic
def send_email(request, cfid):
Expand Down
20 changes: 10 additions & 10 deletionspgcommitfest/urls.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,18 +19,18 @@
re_path(r'^(\d+)/$', views.commitfest),
re_path(r'^(open|inprogress|current)/(.*)$', views.redir),
re_path(r'^(?P<cfid>\d+)/activity(?P<rss>\.rss)?/$', views.activity),
re_path(r'^patch/(\d+)/$', views.patch_redirect),
re_path(r'^(\d+)/(\d+)/$', views.patch),
re_path(r'^(\d+)/(\d+)/edit/$', views.patchform),
re_path(r'^(\d+)/(\d+)/$', views.patch_legacy_redirect),
re_path(r'^patch/(\d+)/$', views.patch),
re_path(r'^patch/(\d+)/edit/$', views.patchform),
re_path(r'^(\d+)/new/$', views.newpatch),
re_path(r'^(\d+)/(\d+)/status/(review|author|committer)/$', views.status),
re_path(r'^(\d+)/(\d+)/close/(reject|withdrawn|feedback|committed|next)/$', views.close),
re_path(r'^(\d+)/(\d+)/reviewer/(become|remove)/$', views.reviewer),
re_path(r'^(\d+)/(\d+)/committer/(become|remove)/$', views.committer),
re_path(r'^(\d+)/(\d+)/(un)?subscribe/$', views.subscribe),
re_path(r'^(\d+)/(\d+)/(comment|review)/', views.comment),
re_path(r'^patch/(\d+)/status/(review|author|committer)/$', views.status),
re_path(r'^patch/(\d+)/close/(reject|withdrawn|feedback|committed|next)/$', views.close),
re_path(r'^patch/(\d+)/reviewer/(become|remove)/$', views.reviewer),
re_path(r'^patch/(\d+)/committer/(become|remove)/$', views.committer),
re_path(r'^patch/(\d+)/(un)?subscribe/$', views.subscribe),
re_path(r'^patch/(\d+)/(comment|review)/', views.comment),
re_path(r'^(\d+)/send_email/$', views.send_email),
re_path(r'^(\d+)/\d+/send_email/$', views.send_email),
re_path(r'^patch/(\d+)/send_email/$', views.send_patch_email),
re_path(r'^(\d+)/reports/authorstats/$', reports.authorstats),
re_path(r'^search/$', views.global_search),
re_path(r'^ajax/(\w+)/$', ajax.main),
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp