Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AD should be a role for groups, not a property of GroupInfo #1557

Closed
ietf-svn-bot opened this issue Dec 10, 2014 · 14 comments
Closed

AD should be a role for groups, not a property of GroupInfo #1557

ietf-svn-bot opened this issue Dec 10, 2014 · 14 comments

Comments

@ietf-svn-bot
Copy link

resolution_fixed type_defect | by rjsparks@nostrum.com


Currently, GroupInfo has an ad field (ForeignKey to Person).

This is a holdover from how Groups worked in older code.
The groups that have ADs, should have a Role record indicating the AD instead.

With the current models, there are groups where a responsible AD doesn't make sense, and the field is either artifically populated or left blank. They also don't allow the notion of more than one AD for a group.

These results are not the most intuitive:

In 0a679b7ff24d8f94560634185d6eca4ea97d90a4: Group.objects.get(acronym='rai').ad

In a00b1631f4c7a6372fdbfd5dbf1bd54853a82bc4: Group.objects.get(acronym='iesg').ad
Outa00b1631f4c7a6372fdbfd5dbf1bd54853a82bc4: <Person: Jari Arkko>

Note that we're already using the Role idea for Areas:

In 98165ab39945f8c997b48d6ecebf5ed05c8d1f90: Group.objects.get(acronym='rai').role_set.all()
Out98165ab39945f8c997b48d6ecebf5ed05c8d1f90: [<Role: Richard Barnes is Area Director in rai>, <Role: Alissa Cooper is Area Director in rai>]

There is code in the views that would be drastically simplified by this change. See, for instance, the view that displays a WG or RG charter.

See also #1555


Issue migrated from trac:1557 at 2022-03-04 03:57:27 +0000

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com edited the issue description

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


From 4c9db8f:

This is the first step towards using Role to represent Area directors. It

  • Migrates the information captured in GroupInfo.ad to Role objects.
  • Renames GroupInfo.ad to GroupInfo._ad (retaining the current column name) to prepare for deletion of that field.
  • Provides ad property accessor and setter methods implemented using the group's role_set (so that the existing view code continues to work with minimal changes)
  • Improved selection in many querysets that assumed only groups of type 'area' would have area directors.

Related to #1557 and #1555.

Commit ready to merge.

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


From 65804be:

This is the second step towards ADs out of GroupInfo into Role.

The use of group.ad has been scrubbed from the code and templates.

  • Those places that set group.ad have been directly manipulate Role objects instead
  • Most places that read group.ad now use a new group.ad_role() that returns a Role object, simplifing some views.

Related to #1555 and #1557.

Commit ready for merge.

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


This is essentially done. What remains is cleanup of the column after we have some runtime.

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


From 65804be:

This is the second step towards ADs out of GroupInfo into Role.

The use of group.ad has been scrubbed from the code and templates.

  • Those places that set group.ad have been directly manipulate Role objects instead
  • Most places that read group.ad now use a new group.ad_role() that returns a Role object, simplifing some views.

Related to #1555 and #1557.

Commit ready for merge.

2 similar comments
@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


From 65804be:

This is the second step towards ADs out of GroupInfo into Role.

The use of group.ad has been scrubbed from the code and templates.

  • Those places that set group.ad have been directly manipulate Role objects instead
  • Most places that read group.ad now use a new group.ad_role() that returns a Role object, simplifing some views.

Related to #1555 and #1557.

Commit ready for merge.

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


From 65804be:

This is the second step towards ADs out of GroupInfo into Role.

The use of group.ad has been scrubbed from the code and templates.

  • Those places that set group.ad have been directly manipulate Role objects instead
  • Most places that read group.ad now use a new group.ad_role() that returns a Role object, simplifing some views.

Related to #1555 and #1557.

Commit ready for merge.

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


Fixed in 56f9260:

Show out-of-area ads on /wg/. Fixes #1555. Related to #1557. Commit ready for merge.

@ietf-svn-bot
Copy link
Author

@henrik@levkowetz.com commented


From edc4cba:

Merged in 4c9db8f from rjsparks@nostrum.com:\n This is the first step towards using Role to represent Area directors. It

@ietf-svn-bot
Copy link
Author

@henrik@levkowetz.com commented


From be1c255:

Merged in 65804be from rjsparks@nostrum.com:
This is the second step towards ADs out of GroupInfo into Role.
The use of group.ad has been scrubbed from the code and templates.

@ietf-svn-bot
Copy link
Author

@henrik@levkowetz.com commented


Fixed in 56f979b:

Merged in 56f9260 from rjsparks@nostrum.com:
Show out-of-area ads on /wg/. Fixes #1555. Related to #1557.

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com changed status from new to closed

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com changed resolution from `` to fixed

@ietf-svn-bot
Copy link
Author

@rjsparks@nostrum.com commented


The cleanup for this was finished with release 5.12.1

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant