PR#4743: lib/query: order pull requests based on updated_on column when we want to order based on last updated time - pagure

pagure

#4743 lib/query: order pull requests based on updated_on column when we want to order based on last updated time

Merged 5 years ago by pingou. Opened 5 years ago by jlanda.

jlanda/pagure fix-pr-order into master

templates/issues.html: render updated_on instead of last_updated on related pull request's info

Julen Landa Alustiza • 5 years ago

10f5fa9

update updated_on on add_pull_request_flag

Julen Landa Alustiza • 5 years ago

fa343dd

lib/query: order pull requests based on updated_on column when we want to order based on last updated time

Julen Landa Alustiza • 5 years ago

6f0ec05

pagure/lib/query.py

file modified

+4 -1

		`@@ -1497,6 +1497,7 @@`
		`user_id=user_obj.id,`
		`token_id=token,`
		`)`
		`+ request.updated_on = datetime.datetime.utcnow()`
		`session.add(pr_flag)`
		`# Make sure we won't have SQLAlchemy error before we continue`
		`session.flush()`
		`@@ -3230,7 +3231,9 @@`
		`column = model.PullRequest.date_created`

		`if order_key == "last_updated":`
		`- column = model.PullRequest.last_updated`
		`+ # We actually want to order on updated_on and not last_updated`
		`+ # https://pagure.io/pagure/issue/4464#comment-624915`
		`+ column = model.PullRequest.updated_on`

		`if requestid:`
		`query = query.filter(model.PullRequest.id == requestid)`

pagure/templates/issue.html

file modified

+1 -1

		`@@ -620,7 +620,7 @@`
		`namespace=pr.project.namespace,`
		`requestid=pr.id) }}">#{{pr.id}}</a>`
		`{{ pr.status if pr.status != 'Open' else 'Last updated'`
		`- }} {{ pr.last_updated \| humanize }}`
		`+ }} {{ pr.updated_on \| humanize }}`
		`</li>`
		`{% endfor %}`
		`</ul>`

tests/test_pagure_flask_ui_fork.py

file modified

+1 -1

		`@@ -1529,7 +1529,7 @@`
		`pr_one = pagure.lib.query.search_pull_requests(`
		`self.session, project_id=1, requestid=1`
		`)`
		`- pr_one.last_updated = datetime.utcnow() + timedelta(seconds=2)`
		`+ pr_one.updated_on = datetime.utcnow() + timedelta(seconds=2)`
		`self.session.add(pr_one)`
		`self.session.commit()`

jlanda commented 5 years ago

Based on https://pagure.io/pagure/issue/4464#comment-624915 , modify the ordering key when we search pull requests with last_updated ordering_key

I choose to keep last_updated as key to keep backward compatibility

jlanda commented 5 years ago

The previous PR included updating the timestamp when a pr was flagged.

I would like to add it here too, but don't know wich of them should I update

rebased onto 6f0ec05

5 years ago

ngompa commented 5 years ago

:thumbsup:

pingou commented 5 years ago

I would like to add it here too, but don't know wich of them should I update

The way I see it, we likely never want to touch last_updated for PRs (and I just realized that for issue last_updated is basically the equivalent of updated_on for PRs, damn this is not going to help to make things clear in the future :s).

jlanda commented 5 years ago

I would like to add it here too, but don't know wich of them should I update

The way I see it, we likely never want to touch last_updated for PRs (and I just realized that for issue last_updated is basically the equivalent of updated_on for PRs, damn this is not going to help to make things clear in the future :s).

And the last_updated api key for order internally is updated_on, Which is nice for external pow since both issues and PRs share key name, but internally is going to confuse even more

Edited 5 years ago by jlanda

1 new commit added