Skip to content

Leave downstream assignment to owner when upstream is clear#380

Merged
ralphbean merged 1 commit intomainfrom
dont-clear-owner-owned
Oct 29, 2025
Merged

Leave downstream assignment to owner when upstream is clear#380
ralphbean merged 1 commit intomainfrom
dont-clear-owner-owned

Conversation

@webbnh
Copy link
Copy Markdown
Collaborator

@webbnh webbnh commented Oct 17, 2025

Currently, if an upstream issue is unassigned, then the downstream issue will be assigned to the project "owner" if one is configured. However, if the upstream issue is updated, and the project is configured to overwrite assignments, then downstream issues assignment will be cleared if the upstream issue is unassigned. This results in an unstable behavior where the downstream issue is alternately assigned to the project owner and then unassigned on each upstream update.

This PR splits the overwrite case from two options to three:

  • when the downstream issue is unassigned, we still call assign_user() unconditionally, and
  • when the upstream issue is assigned, we also still call assign_user(), unless the assignments still match; however,
  • if the upstream issue is unassigned, we now call assign_user() only if the downstream issue is not assigned to the project owner.

This should produce a stable behavior.

This PR also tweaks the log message displayed when we cannot assign the downstream issue so that it shows the upstream assignee's login name if they don't have a "display name".

And, this PR updates the unit test for _update_assignee():

  • It adds another value to the assigned-to dimension: in addition to "none" and "match" it adds "owner".
  • This is a table-driven test which endeavors to test every permutation of possible inputs. This used to be two dozen, but, with the addition of the owner-assigned dimension, this PR raises the count to 36.
  • With this many permutations, it's hard to tell which scenario caused a failure, so this PR adds an "ID" for each permutation. (Unfortunately, this addition made the lines too long for Black's taste, so I had to reformat them.) The ID is reported by means of a try block wrapped around the test assertions (the assertions themselves are otherwise unchanged).
  • For convenience (e.g., in setting conditional breakpoints), this PR moves the determination of the scenario/expected-value to before the invocation of the CUT.

Because the CUT is largely a sequence of boolean expressions rather than branches, all of the statements are executed regardless of the input values, and so only a small set of inputs is required to produce "full coverage"; nevertheless, without testing the full range of inputs, we cannot ensure the proper outputs in all possible cases. So, strictly speaking, the changes to the unit test do not increase the (measured) coverage (Coveralls reports the coverage as increased because I added more lines (which are covered) and not because the improved test covers previously uncovered lines).

@webbnh webbnh force-pushed the dont-clear-owner-owned branch 3 times, most recently from 9148ebe to db4f442 Compare October 24, 2025 21:02
@webbnh webbnh force-pushed the dont-clear-owner-owned branch from db4f442 to 060fb67 Compare October 28, 2025 17:01
@webbnh webbnh marked this pull request as ready for review October 28, 2025 17:22
@ralphbean ralphbean merged commit 9fc7a4b into main Oct 29, 2025
6 checks passed
@ralphbean ralphbean deleted the dont-clear-owner-owned branch October 29, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants