PR#290: Reapply "Sync proposals done through the app instantly" and fix the duplication issue (#286) - fedora-qa/blockerbugs

fedora-qa / blockerbugs

#290 Reapply "Sync proposals done through the app instantly" and fix the duplication issue (#286)

Closed a month ago by kparal. Opened 2 months ago by adamwill.

fedora-qa/ adamwill/blockerbugs fix-dupes into develop

Fix duplication of bugs in `fetch_single_bug` (#286)

Adam Williamson • 2 months ago

4d933b5

Reapply "Sync proposals done through the app instantly"

Adam Williamson • 2 months ago

9044395

blockerbugs/controllers/main.py

file modified

+13 -2

		`@@ -27,8 +27,8 @@`
		`import json`
		`import itertools`

		`- from blockerbugs import app, __version__`
		`- from blockerbugs.util import bz_interface, pagure_bot, misc`
		`+ from blockerbugs import app, db, __version__`
		`+ from blockerbugs.util import bz_interface, pagure_bot, misc, discussion_sync, bug_sync`
		`from blockerbugs.models.bug import Bug`
		`from blockerbugs.models.milestone import Milestone`
		`from blockerbugs.models.update import Update`
		`@@ -443,6 +443,16 @@`

		`return bz_interface.create_bugzilla()`

		`+ def bugzilla_sync_proposal(bugid, milestone, blocker, fe):`
		`+ # Fetch the proposed stuff into the database right away`
		`+ sync = bug_sync.BugSync(db)`
		`+ trackers_str = []`
		`+ if blocker: trackers_str.append("Blocker")`
		`+ if fe: trackers_str.append("FreezeException")`
		`+ sync.fetch_single_bug(milestone, bugid, trackers_str)`
		`+`
		`+ # Create a discussion ticket`
		`+ discussion_sync.create_discussions_links(milestone, [misc.bug_from_db(bugid, milestone)])`

		`@main.route('/propose_bug', methods=['GET', 'POST'])`
		`@fas_login_required`
		`@@ -500,6 +510,7 @@`
		`user, bugid, proposal.get_tracker_type(), selected_milestone.name))`
		`try:`
		`proposal.propose_bugs(user, selected_milestone.name, bugform.justification.data)`
		`+ bugzilla_sync_proposal(bugid, selected_milestone, bool(bugform.blocker.data), bool(bugform.freeze_exception.data))`
		`return render_template('thanks.html', bugid=bugid,`
		`isblocker=bugform.blocker.data,`
		`isfe=bugform.freeze_exception.data,`

blockerbugs/util/bug_sync.py

file modified

+19

		`@@ -150,6 +150,25 @@`

		`self.db.session.commit()`

		`+ def fetch_single_bug(self, milestone: Milestone, bugid, trackers) -> None:`
		`+ self.log.info("Fetching bug data for %s" % bugid)`
		`+ bzbug = self.bzinterface.bz.getbug(bugid)`
		`+ # find an existing bug in the db if there is one`
		`+ dbbug = Bug.query.filter_by(bugid=bug.bug_id, milestone=milestone).first()`
		`+`
		`+ for tracker in trackers:`
		`+ buginfo = self.extract_information(bug, tracker)`
		`+ if dbbug:`
		`+ # update the existing db bug`
		`+ dbbug.update(buginfo, tracker, milestone)`
		`+ self.db.session.add(dbbug)`
		`+ else:`
		`+ # create a new db bug`
		`+ dbbug = Bug.from_data(buginfo, milestone, tracker)`
lbrabec commented a month ago nice catch :thumbsup:
		`+ self.db.session.add(dbbug)`
		`+`
		`+ self.db.session.commit()`
		`+`
		`def cleanup_milestone(self, milestone: Milestone, tracker_type: str) -> None:`
		"""'Clean up' all bugs under `tracker_type` in `milestone`, which are not currently blocking
		`the relevant tracker ticket. Cleaning up means setting all those bugs as rejected`

blockerbugs/util/discussion_sync.py

file modified

+1 -1

		`@@ -2,7 +2,7 @@`

		`from blockerbugs import db, app`
		`from blockerbugs.config import Config as bb_Config`
		`- from blockerbugs.util import pagure_interface`
		`+ from blockerbugs.util import pagure_interface, bz_interface`
		`from blockerbugs.models.milestone import Milestone`
		`from blockerbugs.models.release import Release`
		`from blockerbugs.models.bug import Bug`

blockerbugs/util/misc.py

file modified

+4 -1

		`@@ -3,7 +3,7 @@`
		`from typing import Optional`

		`from blockerbugs import _version`
		`-`
		`+ from blockerbugs.models.bug import Bug`

		`def version_date() -> Optional[str]:`
		`"""Return the date (just the date portion) of the app version identifier, as returned by`
		`@@ -15,3 +15,6 @@`
		`parts = date.split(sep='T')`
		`assert len(parts) == 2`
		`return parts[0]`
		`+`
		`+ def bug_from_db(bugid, milestone):`
		`+ return Bug.query.filter_by(bugid=bugid, milestone=milestone).first()`

adamwill commented 2 months ago

Eyeballing 38ac594 , I'm pretty sure this should fix the dupe db bug entries. The problem is that every iteration of the for tracker in trackers loop in fetch_single_bug creates a new db bug entry, if there was no pre-existing db bug entry. So if a newly-proposed bug is associated with more than one tracker, I'm pretty sure we get dupes.

2 new commits added