Always set one is_lead related org in actions.project.edit.#5731
Open
mikerkelly wants to merge 6 commits intomainfrom
Open
Always set one is_lead related org in actions.project.edit.#5731mikerkelly wants to merge 6 commits intomainfrom
is_lead related org in actions.project.edit.#5731mikerkelly wants to merge 6 commits intomainfrom
Conversation
We ought to set a lead organisation when we edit the project, but we don't. Add lines illlustrating this. The previous version of this test did happen to leave `org1` as lead, because the `ProjectCollaboration` was not removed. So let's make the test use a disjoint set of new orgs. Really there should be more tests.
Various parts of the application do or may assume that there is at least one `org` marked as `is_lead` through `ProjectCollaboration`, per project. So we should ensure that that happens in the edit action. For now, just pick the first that the form provides and set it. This can lead to issues if there was already a lead org set but a different org is passed in first. It happens that right now that cannot happen, as the form only allows one organisation to be set, but we should improve this further. I have left detail in comments as this part of the codebase needs work and has some surprising properties.
Perhaps surprisingly, the lead org can be changed when two orgs are passed in, one of them already being lead org, depending on ordering. This test shows that and will make a future commit that changes this clearer. The behaviour seems a bit too different to parametrise.
For now, this seems the safest thing to do, ensuring that this function always sets a lead org, but does not overwrite it if it was already related, and still is after. This is being defensive against future change, as for now the form can only pass one anyway.
We can create multiple objects in one line, which is briefer and easier to read. `assert_only_lead_org` makes it easier to read what is being asserted about the `ProjectCollaboration` instances.
mikerkelly
force-pushed
the
mikerkelly/project-action-edit-set-is-lead
branch
from
2f8044e to
ff84435
Compare
This makes the label made by the factory include the same number as the variable name number, which is easier to parse when looking at fails or the debugger.
mikerkelly
force-pushed
the
mikerkelly/project-action-edit-set-is-lead
branch
from
ff84435 to
26c4de2
Compare
| assert not Project.objects.exists() | ||
|
|
||
|
|
||
| def assert_only_lead_org(project, org): |
nishtha-kalra
approved these changes
Mar 27, 2026
Contributor
nishtha-kalra
left a comment
nishtha-kalra
left a comment
There was a problem hiding this comment.
I will move discussion about changing the relation to be an foreign key to a new issue.
This PR looks good. I liked the idea that we can make the relation as a foreign key. Could you link the issue to this PR whenever you create it or link this PR to the issue? That way we can always refer this PR whenever someone picks up the issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5699.
The
createaction setsis_leadon a relatedOrgbut theeditone did not. So if you changed the org, none would be lead. This fixes that by setting the first org passed in as lead, if none remain after the update.I will move discussion about changing the relation to be an foreign key to a new issue.