PR#254: make bugimg nicer - fedora-qa/blockerbugs

fedora-qa / blockerbugs

#254 make bugimg nicer

Merged 2 years ago by kparal. Opened 2 years ago by lbrabec.

feature/nicer_bugimg into develop

make bugimg nicer

Lukas Brabec • 2 years ago

8505c5a

blockerbugs/controllers/api/api.py

file modified

+11 -2

		`@@ -196,9 +196,15 @@`
		`bugtypes.append("Accepted FE")`
		`return bugtypes`

		`+`
		`+ def _get_pretty_milestone_name(bug):`
		`+ return "F%d %s: " % (bug.milestone.release.number, bug.milestone.version.capitalize())`
		`+`
		`+`
		`_UNKNOWN_BUG_SVG_TEXT = _svg_response_text(["unknown bug"])`
		`_BUG_CLOSED = "BUG CLOSED"`

		`+`
		`@api_v0.route('/bugimg/<int:bug_id>')`
		`def bug_image(bug_id):`
		`bugs = Bug.query.filter_by(bugid=bug_id).all()`
		`@@ -213,9 +219,12 @@`
		`# show BUG CLOSED when all bug entries in DB have status == "CLOSED"`
		`if all([bug.status == "CLOSED" for bug in bugs]):`
		`info_all.append(_BUG_CLOSED)`
		`- for bug in bugs:`
		`- milestone_info = "%s: " % bug.milestone.name`
		`+ for bug in sorted(bugs, key=lambda b: b.milestone.name):`
		`+ milestone_info = _get_pretty_milestone_name(bug)`
		`bugtypes = _get_bugtypes(bug)`
		`+ if not bugtypes:`
		`+ # skip this info line if empty string is returned`
		`+ continue`
		`info_all.append(milestone_info + ", ".join(bugtypes))`

		`return SVGResponse(_svg_response_text(info_all))`

testing/test_api.py

file modified

+31 -4

		`@@ -7,12 +7,14 @@`

		`from blockerbugs import db`
		`from blockerbugs import app`
		`+ from blockerbugs.models.milestone import Milestone`
		`from blockerbugs.models.bug import Bug`
		`from blockerbugs.models.userinfo import UserInfo`
		`from testing.test_controllers import add_release, add_milestone, \`
		`add_bug, add_update`
		`from blockerbugs.controllers.api import api, errors`
		`- from blockerbugs.controllers.api.api import _get_bugtypes, _UNKNOWN_BUG_SVG_TEXT, _BUG_CLOSED`
		`+ from blockerbugs.controllers.api.api import _get_bugtypes, _get_pretty_milestone_name, \`
		`+ _UNKNOWN_BUG_SVG_TEXT, _BUG_CLOSED`
		`from blockerbugs.util import pagure_bot`


		`@@ -22,6 +24,8 @@`
		`db.session.rollback()`
		`db.drop_all()`
		`db.create_all()`
		`+ s = db.session()`
		`+ s.expire_on_commit = False # avoid LazyLoading error, https://docs.sqlalchemy.org/en/14/errors.html#error-bhk3`
		`cls.client = app.test_client()`
		`cls.release = add_release(99)`
		`cls.milestone = add_milestone(cls.release, 'final', 100, 101,`
		`@@ -40,6 +44,9 @@`
		`bug2 = add_bug(9002, 'testbug2', cls.milestone)`
		`bug2.accepted_blocker = False`
		`bug2.proposed_fe = True`
		`+ bug2copy = add_bug(9002, 'testbug2', cls.milestone2) # different milestone than bug2`
		`+ bug2copy.accepted_blocker = False`
		`+ bug2copy.proposed_fe = True`
		`bug3 = add_bug(9003, 'testbug3', cls.milestone)`
		`bug3.accepted_blocker = False`
		`bug3.proposed_fe = True`
		`@@ -173,7 +180,8 @@`

		`bug2 = db.session.query(Bug).filter_by(bugid = 9002).first()`

		`- assert self.milestone.name in data`
		`+ pretty_name = _get_pretty_milestone_name(bug2)`
		`+ assert pretty_name in data`
		`for bugtype in _get_bugtypes(bug2):`
		`assert bugtype in data`

		`@@ -194,6 +202,23 @@`

		`assert _BUG_CLOSED not in data`

		`+ def test_get_bugimg_multiple_milestones_sorted(self):`
		`+ url = '/api/v0/bugimg/9002'`
		`+ resp = self.client.get(url)`
		`+ assert resp.status_code == httplib.OK`
		`+ data = str(resp.data)`
		`+`
		`+ beta_milestone = db.session.query(Milestone).filter_by(version = 'beta').first()`
		`+ bug2beta = db.session.query(Bug).filter_by(bugid = 9002, milestone = beta_milestone).first()`
		`+`
		`+ final_milestone = db.session.query(Milestone).filter_by(version = 'final').first()`
		`+ bug2final = db.session.query(Bug).filter_by(bugid = 9002, milestone = final_milestone).first()`
		`+`
		`+ pretty_name_beta = _get_pretty_milestone_name(bug2beta)`
		`+ pretty_name_final = _get_pretty_milestone_name(bug2final)`
		`+`
		`+ assert data.index(pretty_name_beta) < data.index(pretty_name_final)`
		`+`
		`def test_get_bugimg_closed_bug(self):`
		`url = '/api/v0/bugimg/9000'`
		`resp = self.client.get(url)`
		`@@ -208,8 +233,10 @@`
		`assert resp.status_code == httplib.OK`
		`data = str(resp.data)`

		`- assert '99-beta' in data`
		`- assert '99-final' in data`
		`+ bugs = db.session.query(Bug).filter_by(bugid = 9003).all()`
		`+`
		`+ for bug in bugs:`
		`+ assert _get_pretty_milestone_name(bug) in data`
		`assert _BUG_CLOSED not in data`

		`# === /api/v0/webhook ===`

lbrabec commented 2 years ago

"F37 Beta" instead of "37-beta"
milestones are sorted instead of a random order

fix for #247

kparal commented 2 years ago

New tests! Me likes! :-)

Thanks for the patch. There's one slight issue that sometimes it generates empty lines, which is visible even in the screenshot in #247. I believe it's related to bugs which are only marked as Prioritized in the given milestone. Then _get_bugtypes() returns an empty string (which is OK, I think, we don't need to display "Prioritized" for our purposes). In this case, the output line should be skipped. WDYT? Can you please adjust that?

rebased onto 8505c5a

2 years ago