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

feat: add group leadership list #8135

Merged
merged 5 commits into from
Nov 15, 2024
Merged

feat: add group leadership list #8135

merged 5 commits into from
Nov 15, 2024

Conversation

rpcross
Copy link
Collaborator

@rpcross rpcross commented Nov 3, 2024

No description provided.

Copy link
Member

@jennifer-richards jennifer-richards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests should probably check that rg groups are handled correctly. As it stands, I think rg groups could be ignored and the tests would still pass. (RoleFactory(group__type_id="rg", name_id="chair") might be a useful tool for that.

It'd also be nice to test that active/bof/proposed groups are included but, e.g., conclude groups are ignored. Here, RoleFactory(..., group__state_id="conclude", ...) might help.

FYI, the existing group you're seeing ("Sops") is set up in ietf/utils/test_data.py.

Suggest running the Black styler over the new code. (Or at least using double-quotes instead of single)

ietf/group/views.py Show resolved Hide resolved
ietf/templates/group/group_leadership.html Outdated Show resolved Hide resolved
ietf/templates/group/group_leadership.html Outdated Show resolved Hide resolved

def get_leadership(group_type):
people = Person.objects.filter(
role__name__slug="chair",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not asking for a change now, but do think about the future when we might want to add IAB programs to these pages - we'll need to look for other role names at that point.

Copy link
Member

@jennifer-richards jennifer-richards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes!

Copy link

codecov bot commented Nov 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.94%. Comparing base (c7f6bde) to head (9d3deca).
Report is 184 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8135      +/-   ##
==========================================
+ Coverage   88.78%   88.94%   +0.15%     
==========================================
  Files         296      303       +7     
  Lines       41320    41284      -36     
==========================================
+ Hits        36687    36720      +33     
+ Misses       4633     4564      -69     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rjsparks rjsparks merged commit b07d4db into ietf-tools:main Nov 15, 2024
9 checks passed
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.

3 participants