Differential D1079

Resultsdb - fix the pagination issue
ClosedPublic
Actions

Authored by jskladan on Jan 12 2017, 1:18 PM.

Details

Reviewers
kparal
Group Reviewers
resultsdb
Maniphest Tasks
T877: resultsdb_frontend: "Older results" link ignores current query
Commits
rRSDBd7f7ad945211: Resultsdb - fix the pagination issue
Summary

When page=xyz was not present in the URL the next/previous URLs were sometimes produced in the wrong way.
Instead of checking for presence of other URL-encoded attributes ?page=1 was blindly added to the end of the URL string, while sometimes, it should have been &page=1.
This patch fixes that.

Test Plan

Adjusted unittests

Diff Detail

Repository
rRSDB resultsdb
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jskladan retitled this revision from to Resultsdb - fix the pagination issue.Jan 12 2017, 1:18 PM
jskladan updated this object.
jskladan edited the test plan for this revision. (Show Details)
jskladan added a reviewer: resultsdb.
kparal accepted this revision.Jan 13 2017, 1:25 PM
kparal added a reviewer: kparal.
kparal added a subscriber: kparal.

That looks like it should fix it. Thanks!

This revision is now accepted and ready to land.Jan 13 2017, 1:25 PM
Closed by commit rRSDBd7f7ad945211: Resultsdb - fix the pagination issue (authored by Josef Skladanka <jskladan@redhat.com>). · Explain WhyJan 13 2017, 3:31 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 2750

View Options

resultsdb/controllers/api_v2.py

Show First 20 Lines • Show All 118 Lines • ▼ Show 20 Line(s)113def prev_next_urls(data, limit=QUERY_LIMIT):
119 except IndexError: # page not found 119​ except IndexError: # page not found
120 page = None 120​ page = None
121 121​
122 prev = None 122​ prev = None
123 next = None 123​ next = None
124 placeholder = "[!@#$%^&*PLACEHOLDER*&^%$#@!]" 124​ placeholder = "[!@#$%^&*PLACEHOLDER*&^%$#@!]"
125 125​
126 if not page: 126​ if not page:
127 baseurl = "%s?page=%s" % (request.url, placeholder) 127​ if "?" in request.url:
128​ baseurl = "%s&page=%s" % (request.url, placeholder)
129​ else:
130​ baseurl = "%s?page=%s" % (request.url, placeholder)
128 page = 0 131​ page = 0
129 else: 132​ else:
130 baseurl = RE_PAGE.sub("%spage=%s" % (flag, placeholder), request.url) 133​ baseurl = RE_PAGE.sub("%spage=%s" % (flag, placeholder), request.url)
131 134​
132 if page > 0: 135​ if page > 0:
133 prev = baseurl.replace(placeholder, str(page - 1)) 136​ prev = baseurl.replace(placeholder, str(page - 1))
134 if len(data) > limit: 137​ if len(data) > limit:
135 next = baseurl.replace(placeholder, str(page + 1)) 138​ next = baseurl.replace(placeholder, str(page + 1))
▲ Show 20 Lines • Show All 579 Lines • Show Last 20 Lines
View Options

testing/test_helpers.py

Show First 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s)45 def test_data_no_page_in_url(self, monkeypatch):
46 self.rq.url = 'URL' 46​ self.rq.url = 'URL'
47 monkeypatch.setattr(apiv2, 'request', self.rq) 47​ monkeypatch.setattr(apiv2, 'request', self.rq)
48 48​
49 data, prev, next = apiv2.prev_next_urls(range(10), 1) 49​ data, prev, next = apiv2.prev_next_urls(range(10), 1)
50 assert data == [0] 50​ assert data == [0]
51 assert prev is None 51​ assert prev is None
52 assert next == 'URL?page=1' 52​ assert next == 'URL?page=1'
53 53​
54​ def test_data_no_page_in_url_stuff_in_url(self, monkeypatch):
55​ self.rq.url = 'URL?stuff=some'
56​ monkeypatch.setattr(apiv2, 'request', self.rq)
57​
58​ data, prev, next = apiv2.prev_next_urls(range(10), 1)
59​ assert data == [0]
60​ assert prev is None
61​ assert next == 'URL?stuff=some&page=1'
62​
54 def test_data_page_and_limit_in_url(self, monkeypatch): 63​ def test_data_page_and_limit_in_url(self, monkeypatch):
55 self.rq.url = 'URL?page=1&limit=1' 64​ self.rq.url = 'URL?page=1&limit=1'
56 monkeypatch.setattr(apiv2, 'request', self.rq) 65​ monkeypatch.setattr(apiv2, 'request', self.rq)
57 66​
58 data, prev, next = apiv2.prev_next_urls(range(10), 1) 67​ data, prev, next = apiv2.prev_next_urls(range(10), 1)
59 assert data == [0] 68​ assert data == [0]
60 assert prev == 'URL?page=0&limit=1' 69​ assert prev == 'URL?page=0&limit=1'
61 assert next == 'URL?page=2&limit=1' 70​ assert next == 'URL?page=2&limit=1'
62 71​
72​ self.rq.url = 'URL?limit=1&page=1'
73​ monkeypatch.setattr(apiv2, 'request', self.rq)
74​
75​ data, prev, next = apiv2.prev_next_urls(range(10), 1)
76​ assert data == [0]
77​ assert prev == 'URL?limit=1&page=0'
78​ assert next == 'URL?limit=1&page=2'
79​
63 self.rq.url = 'URL&page=1&limit=1' 80​ self.rq.url = 'URL&page=1&limit=1'
64 monkeypatch.setattr(apiv2, 'request', self.rq) 81​ monkeypatch.setattr(apiv2, 'request', self.rq)
65 82​
66 data, prev, next = apiv2.prev_next_urls(range(10), 1) 83​ data, prev, next = apiv2.prev_next_urls(range(10), 1)
67 assert data == [0] 84​ assert data == [0]
68 assert prev == 'URL&page=0&limit=1' 85​ assert prev == 'URL&page=0&limit=1'
69 assert next == 'URL&page=2&limit=1' 86​ assert next == 'URL&page=2&limit=1'
70 87​
88​ self.rq.url = 'URL&limit=1&page=1'
89​ monkeypatch.setattr(apiv2, 'request', self.rq)
90​
91​ data, prev, next = apiv2.prev_next_urls(range(10), 1)
92​ assert data == [0]
93​ assert prev == 'URL&limit=1&page=0'
94​ assert next == 'URL&limit=1&page=2'
95​
71 96​
72class TestParseSince(): 97​class TestParseSince():
73 98​
74 def setup_method(self, method): 99​ def setup_method(self, method):
75 self.date_str = '2016-01-01T01:02:03.04' 100​ self.date_str = '2016-01-01T01:02:03.04'
76 self.date_obj = datetime.datetime.strptime(self.date_str, "%Y-%m-%dT%H:%M:%S.%f") 101​ self.date_obj = datetime.datetime.strptime(self.date_str, "%Y-%m-%dT%H:%M:%S.%f")
77 102​
78 def test_parse_start(self): 103​ def test_parse_start(self):
Show All 22 Lines