diff --git a/backend/apps/github/api/internal/nodes/user.py b/backend/apps/github/api/internal/nodes/user.py index c305d7764e..c709432f33 100644 --- a/backend/apps/github/api/internal/nodes/user.py +++ b/backend/apps/github/api/internal/nodes/user.py @@ -13,12 +13,10 @@ "avatar_url", "bio", "company", - "contributions_count", "email", "followers_count", "following_count", "id", - "is_owasp_staff", "location", "login", "name", @@ -28,6 +26,14 @@ class UserNode: """GitHub user node.""" + def _resolve_contributions_count(self) -> int: + """Resolve contributions count.""" + if hasattr(self, "owasp_profile"): + return self.owasp_profile.contributions_count + return super().__getattribute__("contributions_count") + + contributions_count: int = strawberry.field(resolver=_resolve_contributions_count) + @strawberry.field def badge_count(self) -> int: """Resolve badge count.""" @@ -83,6 +89,14 @@ def is_gsoc_mentor(self) -> bool: return self.owasp_profile.is_gsoc_mentor return False + def _resolve_is_owasp_staff(self) -> bool: + """Resolve if the user is an OWASP staff member.""" + if hasattr(self, "owasp_profile"): + return self.owasp_profile.is_owasp_staff + return super().__getattribute__("is_owasp_staff") + + is_owasp_staff: bool = strawberry.field(resolver=_resolve_is_owasp_staff) + @strawberry.field def issues_count(self) -> int: """Resolve issues count.""" diff --git a/backend/apps/github/api/internal/queries/user.py b/backend/apps/github/api/internal/queries/user.py index 199c33aa90..95756c03b8 100644 --- a/backend/apps/github/api/internal/queries/user.py +++ b/backend/apps/github/api/internal/queries/user.py @@ -50,4 +50,11 @@ def user( User or None: The user object if found, otherwise None. """ + if user := ( + User.objects.select_related("owasp_profile") + .filter(owasp_profile__has_public_member_page=True, login=login) + .first() + ): + return user + return User.objects.filter(has_public_member_page=True, login=login).first() diff --git a/backend/apps/github/management/commands/github_update_users.py b/backend/apps/github/management/commands/github_update_users.py index 42a6e51e4d..a1a440719c 100644 --- a/backend/apps/github/management/commands/github_update_users.py +++ b/backend/apps/github/management/commands/github_update_users.py @@ -8,6 +8,7 @@ from apps.common.models import BATCH_SIZE from apps.github.models.repository_contributor import RepositoryContributor from apps.github.models.user import User +from apps.owasp.models.member_profile import MemberProfile logger = logging.getLogger(__name__) @@ -45,15 +46,39 @@ def handle(self, *args, **options): .values("user_id") .annotate(total_contributions=Sum("contributions_count")) } + profiles = [] users = [] for idx, user in enumerate(active_users[offset:]): prefix = f"{idx + offset + 1} of {active_users_count - offset}" print(f"{prefix:<10} {user.title}") - user.contributions_count = user_contributions.get(user.id, 0) + profile, created = MemberProfile.objects.get_or_create(github_user=user) + if created: + profile.github_user = user + contributions = user_contributions.get(user.id, 0) + profile.contributions_count = contributions + profiles.append(profile) + + user.contributions_count = contributions users.append(user) - if not len(users) % BATCH_SIZE: - User.bulk_save(users, fields=("contributions_count",)) + if not len(profiles) % BATCH_SIZE: + MemberProfile.bulk_save( + profiles, + fields=("contributions_count",), + ) + User.bulk_save( + users, + fields=("contributions_count",), + ) + if profiles: + MemberProfile.bulk_save( + profiles, + fields=("contributions_count",), + ) - User.bulk_save(users, fields=("contributions_count",)) + if users: + User.bulk_save( + users, + fields=("contributions_count",), + ) diff --git a/backend/apps/github/models/mixins/user.py b/backend/apps/github/models/mixins/user.py index bdbb2c2f19..862d2e0f51 100644 --- a/backend/apps/github/models/mixins/user.py +++ b/backend/apps/github/models/mixins/user.py @@ -124,6 +124,8 @@ def idx_contributions(self): @property def idx_contributions_count(self) -> int: """Return contributions count for indexing.""" + if hasattr(self, "owasp_profile") and self.owasp_profile.contributions_count: + return self.owasp_profile.contributions_count return self.contributions_count @property diff --git a/backend/apps/nest/api/internal/nodes/user.py b/backend/apps/nest/api/internal/nodes/user.py index ab56a4f5c6..ccb6c3cda1 100644 --- a/backend/apps/nest/api/internal/nodes/user.py +++ b/backend/apps/nest/api/internal/nodes/user.py @@ -13,4 +13,6 @@ class AuthUserNode(strawberry.relay.Node): @strawberry_django.field def is_owasp_staff(self) -> bool: """Check if the user is an OWASP staff member.""" - return self.github_user.is_owasp_staff if self.github_user else False + if hasattr(self.github_user, "owasp_profile"): + return self.github_user.owasp_profile.is_owasp_staff + return self.github_user.is_owasp_staff diff --git a/backend/apps/nest/management/commands/nest_update_badges.py b/backend/apps/nest/management/commands/nest_update_badges.py index 209313134c..4a3a27ce0a 100644 --- a/backend/apps/nest/management/commands/nest_update_badges.py +++ b/backend/apps/nest/management/commands/nest_update_badges.py @@ -3,6 +3,7 @@ import logging from django.core.management.base import BaseCommand +from django.db.models import Q from apps.github.models.user import User from apps.nest.models.badge import Badge @@ -42,7 +43,8 @@ def update_owasp_staff_badge(self): # Assign badge to employees who don't have it. employees_without_badge = User.objects.filter( - is_owasp_staff=True, + Q(owasp_profile__is_owasp_staff=True) + | Q(is_owasp_staff=True, owasp_profile__isnull=True), ).exclude( user_badges__badge=badge, ) @@ -60,7 +62,8 @@ def update_owasp_staff_badge(self): # Remove badge from non-OWASP employees. non_employees = User.objects.filter( - is_owasp_staff=False, + Q(owasp_profile__is_owasp_staff=False) + | Q(is_owasp_staff=False, owasp_profile__isnull=True), user_badges__badge=badge, ).distinct() removed_count = non_employees.count() diff --git a/backend/apps/owasp/admin/member_profile.py b/backend/apps/owasp/admin/member_profile.py index 9e5ec7ddaa..8851a4a9a5 100644 --- a/backend/apps/owasp/admin/member_profile.py +++ b/backend/apps/owasp/admin/member_profile.py @@ -11,6 +11,9 @@ class MemberProfileAdmin(admin.ModelAdmin): autocomplete_fields = ("github_user",) list_display = ( "github_user", + "is_owasp_staff", + "has_public_member_page", + "contributions_count", "owasp_slack_id", "first_contribution_at", "is_owasp_board_member", @@ -47,7 +50,14 @@ class MemberProfileAdmin(admin.ModelAdmin): ), ( "Contribution Information", - {"fields": ("first_contribution_at",)}, + { + "fields": ( + "first_contribution_at", + "is_owasp_staff", + "has_public_member_page", + "contributions_count", + ) + }, ), ( "Membership Flags", diff --git a/backend/apps/owasp/api/internal/permissions/project_health_metrics.py b/backend/apps/owasp/api/internal/permissions/project_health_metrics.py index aa75f689ec..fdb33555e3 100644 --- a/backend/apps/owasp/api/internal/permissions/project_health_metrics.py +++ b/backend/apps/owasp/api/internal/permissions/project_health_metrics.py @@ -10,8 +10,11 @@ class HasDashboardAccess(BasePermission): def has_permission(self, source, info, **kwargs) -> bool: """Check if the user has dashboard access.""" - return ( - (user := info.context.request.user) - and user.is_authenticated - and user.github_user.is_owasp_staff - ) + user = info.context.request.user + if not (user and user.is_authenticated and user.github_user): + return False + + if hasattr(user.github_user, "owasp_profile"): + return user.github_user.owasp_profile.is_owasp_staff + + return user.github_user.is_owasp_staff diff --git a/backend/apps/owasp/api/internal/views/permissions.py b/backend/apps/owasp/api/internal/views/permissions.py index c193e0c985..53053e682b 100644 --- a/backend/apps/owasp/api/internal/views/permissions.py +++ b/backend/apps/owasp/api/internal/views/permissions.py @@ -7,7 +7,15 @@ def has_dashboard_permission(request): """Check if user has dashboard access.""" - return (user := request.user) and user.is_authenticated and user.github_user.is_owasp_staff + user = request.user + if not (user and user.is_authenticated and hasattr(user, "github_user") and user.github_user): + return False + + github_user = user.github_user + if hasattr(github_user, "owasp_profile"): + return github_user.owasp_profile.is_owasp_staff + + return github_user.is_owasp_staff def dashboard_access_required(view_func): diff --git a/backend/apps/owasp/migrations/0066_memberprofile_contributions_count_and_more.py b/backend/apps/owasp/migrations/0066_memberprofile_contributions_count_and_more.py new file mode 100644 index 0000000000..a03d95c3d5 --- /dev/null +++ b/backend/apps/owasp/migrations/0066_memberprofile_contributions_count_and_more.py @@ -0,0 +1,31 @@ +# Generated by Django 5.2.7 on 2025-11-18 17:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("owasp", "0065_memberprofile_linkedin_page_id"), + ] + + operations = [ + migrations.AddField( + model_name="memberprofile", + name="contributions_count", + field=models.PositiveIntegerField(default=0, verbose_name="Contributions count"), + ), + migrations.AddField( + model_name="memberprofile", + name="has_public_member_page", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="memberprofile", + name="is_owasp_staff", + field=models.BooleanField( + default=False, + help_text="Indicates if the user is OWASP Foundation staff.", + verbose_name="Is OWASP Staff", + ), + ), + ] diff --git a/backend/apps/owasp/migrations/0067_memberprofile_backward_compatibility.py b/backend/apps/owasp/migrations/0067_memberprofile_backward_compatibility.py new file mode 100644 index 0000000000..9fd8eb1275 --- /dev/null +++ b/backend/apps/owasp/migrations/0067_memberprofile_backward_compatibility.py @@ -0,0 +1,43 @@ +# Generated by Django 5.2.7 on 2025-11-18 18:04 + +from django.db import migrations + + +def copy_user_data_to_member_profile(apps, _schema_editor): + """Copy user data to member profile.""" + User = apps.get_model("github", "User") + MemberProfile = apps.get_model("owasp", "MemberProfile") + profiles_to_update = [] + batch_size = 500 + update_fields = [ + "has_public_member_page", + "is_owasp_staff", + "contributions_count", + ] + + for user in User.objects.all().iterator(chunk_size=batch_size): + profile, _ = MemberProfile.objects.get_or_create(github_user=user) + profile.has_public_member_page = user.has_public_member_page + profile.is_owasp_staff = user.is_owasp_staff + profile.contributions_count = user.contributions_count + profiles_to_update.append(profile) + + if len(profiles_to_update) >= batch_size: + MemberProfile.objects.bulk_update( + profiles_to_update, update_fields, batch_size=batch_size + ) + profiles_to_update = [] + + if profiles_to_update: + MemberProfile.objects.bulk_update(profiles_to_update, update_fields, batch_size=batch_size) + + +class Migration(migrations.Migration): + dependencies = [ + ("owasp", "0066_memberprofile_contributions_count_and_more"), + ("github", "0039_remove_commit_commit_repo_created_idx"), + ] + + operations = [ + migrations.RunPython(copy_user_data_to_member_profile, migrations.RunPython.noop), + ] diff --git a/backend/apps/owasp/migrations/0068_alter_memberprofile_has_public_member_page.py b/backend/apps/owasp/migrations/0068_alter_memberprofile_has_public_member_page.py new file mode 100644 index 0000000000..bd844150d3 --- /dev/null +++ b/backend/apps/owasp/migrations/0068_alter_memberprofile_has_public_member_page.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.8 on 2025-11-26 14:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("owasp", "0067_memberprofile_backward_compatibility"), + ] + + operations = [ + migrations.AlterField( + model_name="memberprofile", + name="has_public_member_page", + field=models.BooleanField( + default=True, + help_text="Whether the member's profile is publicly visible on the OWASP website", + verbose_name="Has Public Member Page", + ), + ), + ] diff --git a/backend/apps/owasp/models/member_profile.py b/backend/apps/owasp/models/member_profile.py index ba52ed5a1d..546e565d2c 100644 --- a/backend/apps/owasp/models/member_profile.py +++ b/backend/apps/owasp/models/member_profile.py @@ -5,7 +5,7 @@ from django.core.validators import RegexValidator from django.db import models -from apps.common.models import TimestampedModel +from apps.common.models import BulkSaveModel, TimestampedModel from apps.github.models.user import User @@ -71,6 +71,25 @@ class Meta: help_text="LinkedIn username or custom URL ID (e.g., 'john-doe-123')", ) + has_public_member_page = models.BooleanField( + default=True, + verbose_name="Has Public Member Page", + help_text="Whether the member's profile is publicly visible on the OWASP website", + ) + is_owasp_staff = models.BooleanField( + default=False, + verbose_name="Is OWASP Staff", + help_text="Indicates if the user is OWASP Foundation staff.", + ) + contributions_count = models.PositiveIntegerField( + verbose_name="Contributions count", default=0 + ) + def __str__(self) -> str: """Return human-readable representation.""" return f"OWASP member profile for {self.github_user.login}" + + @staticmethod + def bulk_save(profiles, fields=None) -> None: + """Bulk save member profiles.""" + BulkSaveModel.bulk_save(MemberProfile, profiles, fields=fields) diff --git a/backend/tests/apps/github/api/internal/queries/user_test.py b/backend/tests/apps/github/api/internal/queries/user_test.py index 4b430d030b..0a203d5ba7 100644 --- a/backend/tests/apps/github/api/internal/queries/user_test.py +++ b/backend/tests/apps/github/api/internal/queries/user_test.py @@ -14,51 +14,52 @@ def mock_user(self): """User mock fixture.""" return Mock(spec=User) - def test_resolve_user_existing_with_public_member_page(self, mock_user): - """Test resolving an existing user with has_public_member_page=True.""" - with patch("apps.github.models.user.User.objects.filter") as mock_filter: - mock_queryset = mock_filter.return_value - mock_queryset.first.return_value = mock_user - - result = UserQuery().user(login="test-user") - - assert result == mock_user - mock_filter.assert_called_once_with(has_public_member_page=True, login="test-user") - mock_queryset.first.assert_called_once() - - def test_resolve_user_not_found_when_has_public_member_page_false(self): - """Test resolving a user with has_public_member_page=False returns None.""" - with patch("apps.github.models.user.User.objects.filter") as mock_filter: - mock_queryset = mock_filter.return_value - mock_queryset.first.return_value = None - - result = UserQuery().user(login="test-user") - - assert result is None - mock_filter.assert_called_once_with(has_public_member_page=True, login="test-user") - mock_queryset.first.assert_called_once() - - def test_resolve_user_not_found(self): - """Test resolving a non-existent user.""" - with patch("apps.github.models.user.User.objects.filter") as mock_filter: - mock_queryset = mock_filter.return_value - mock_queryset.first.return_value = None - - result = UserQuery().user(login="non-existent") - - assert result is None - mock_filter.assert_called_once_with(has_public_member_page=True, login="non-existent") - mock_queryset.first.assert_called_once() - - def test_resolve_user_filters_by_public_member_page_and_login(self): - """Test that user query filters by both has_public_member_page and login.""" - with patch("apps.github.models.user.User.objects.filter") as mock_filter: - mock_queryset = mock_filter.return_value - mock_queryset.first.return_value = None - - UserQuery().user(login="test-user") - - mock_filter.assert_called_once_with(has_public_member_page=True, login="test-user") + @patch("apps.github.models.user.User.objects.filter") + @patch("apps.github.models.user.User.objects.select_related") + def test_resolve_user_found_on_first_query(self, mock_select_related, mock_filter, mock_user): + """Test resolving an existing user on the first query.""" + mock_select_related.return_value.filter.return_value.first.return_value = mock_user + + result = UserQuery().user(login="test-user") + + assert result == mock_user + mock_select_related.assert_called_once_with("owasp_profile") + mock_select_related.return_value.filter.assert_called_once_with( + owasp_profile__has_public_member_page=True, login="test-user" + ) + mock_filter.assert_not_called() + + @patch("apps.github.models.user.User.objects.filter") + @patch("apps.github.models.user.User.objects.select_related") + def test_resolve_user_found_on_second_query(self, mock_select_related, mock_filter, mock_user): + """Test resolving an existing user on the second (fallback) query.""" + mock_select_related.return_value.filter.return_value.first.return_value = None + mock_filter.return_value.first.return_value = mock_user + + result = UserQuery().user(login="test-user") + + assert result == mock_user + mock_select_related.assert_called_once_with("owasp_profile") + mock_select_related.return_value.filter.assert_called_once_with( + owasp_profile__has_public_member_page=True, login="test-user" + ) + mock_filter.assert_called_once_with(has_public_member_page=True, login="test-user") + + @patch("apps.github.models.user.User.objects.filter") + @patch("apps.github.models.user.User.objects.select_related") + def test_resolve_user_not_found(self, mock_select_related, mock_filter): + """Test resolving a non-existent user returns None.""" + mock_select_related.return_value.filter.return_value.first.return_value = None + mock_filter.return_value.first.return_value = None + + result = UserQuery().user(login="non-existent") + + assert result is None + mock_select_related.assert_called_once_with("owasp_profile") + mock_select_related.return_value.filter.assert_called_once_with( + owasp_profile__has_public_member_page=True, login="non-existent" + ) + mock_filter.assert_called_once_with(has_public_member_page=True, login="non-existent") def test_top_contributed_repositories(self): """Test resolving top contributed repositories.""" diff --git a/backend/tests/apps/github/management/commands/github_update_users_test.py b/backend/tests/apps/github/management/commands/github_update_users_test.py index 2f4f7e6080..0361a419ef 100644 --- a/backend/tests/apps/github/management/commands/github_update_users_test.py +++ b/backend/tests/apps/github/management/commands/github_update_users_test.py @@ -30,14 +30,31 @@ def test_add_arguments(self): "--offset", default=0, required=False, type=int ) + @patch("apps.github.management.commands.github_update_users.MemberProfile") @patch("apps.github.management.commands.github_update_users.User") @patch("apps.github.management.commands.github_update_users.RepositoryContributor") @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 2) - def test_handle_with_default_offset(self, mock_repository_contributor, mock_user): + def test_handle_with_default_offset( + self, mock_repository_contributor, mock_user, mock_member_profile + ): """Test command execution with default offset.""" - mock_user1 = MagicMock(id=1, title="User 1", contributions_count=0) - mock_user2 = MagicMock(id=2, title="User 2", contributions_count=0) - mock_user3 = MagicMock(id=3, title="User 3", contributions_count=0) + mock_profile1 = MagicMock(contributions_count=0) + mock_profile2 = MagicMock(contributions_count=0) + mock_profile3 = MagicMock(contributions_count=0) + + def get_or_create_side_effect(github_user): + if github_user.id == 1: + return mock_profile1, False + if github_user.id == 2: + return mock_profile2, False + if github_user.id == 3: + return mock_profile3, False + return MagicMock(), False + + mock_member_profile.objects.get_or_create.side_effect = get_or_create_side_effect + mock_user1 = MagicMock(id=1, title="User 1") + mock_user2 = MagicMock(id=2, title="User 2") + mock_user3 = MagicMock(id=3, title="User 3") mock_users_queryset = MagicMock() mock_users_queryset.count.return_value = 3 @@ -69,20 +86,38 @@ def test_handle_with_default_offset(self, mock_repository_contributor, mock_user mock_print.assert_any_call("2 of 3 User 2") mock_print.assert_any_call("3 of 3 User 3") - assert mock_user1.contributions_count == 10 - assert mock_user2.contributions_count == 20 - assert mock_user3.contributions_count == 30 + assert mock_profile1.contributions_count == 10 + assert mock_profile2.contributions_count == 20 + assert mock_profile3.contributions_count == 30 - assert mock_user.bulk_save.call_count == 2 - assert mock_user.bulk_save.call_args_list[-1][0][0] == [mock_user1, mock_user2, mock_user3] + assert mock_member_profile.bulk_save.call_count == 2 + assert mock_member_profile.bulk_save.call_args_list[-1][0][0] == [ + mock_profile1, + mock_profile2, + mock_profile3, + ] + @patch("apps.github.management.commands.github_update_users.MemberProfile") @patch("apps.github.management.commands.github_update_users.User") @patch("apps.github.management.commands.github_update_users.RepositoryContributor") @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 2) - def test_handle_with_custom_offset(self, mock_repository_contributor, mock_user): + def test_handle_with_custom_offset( + self, mock_repository_contributor, mock_user, mock_member_profile + ): """Test command execution with custom offset.""" - mock_user1 = MagicMock(id=2, title="User 2", contributions_count=0) - mock_user2 = MagicMock(id=3, title="User 3", contributions_count=0) + mock_profile1 = MagicMock(contributions_count=0) + mock_profile2 = MagicMock(contributions_count=0) + + def get_or_create_side_effect(github_user): + if github_user.id == 2: + return mock_profile1, False + if github_user.id == 3: + return mock_profile2, False + return MagicMock(), False + + mock_member_profile.objects.get_or_create.side_effect = get_or_create_side_effect + mock_user1 = MagicMock(id=2, title="User 2") + mock_user2 = MagicMock(id=3, title="User 3") mock_users_queryset = MagicMock() mock_users_queryset.count.return_value = 3 @@ -109,21 +144,35 @@ def test_handle_with_custom_offset(self, mock_repository_contributor, mock_user) mock_print.assert_any_call("2 of 2 User 2") mock_print.assert_any_call("3 of 2 User 3") - assert mock_user1.contributions_count == 20 - assert mock_user2.contributions_count == 30 + assert mock_profile1.contributions_count == 20 + assert mock_profile2.contributions_count == 30 - assert mock_user.bulk_save.call_count == 2 - assert mock_user.bulk_save.call_args_list[-1][0][0] == [mock_user1, mock_user2] + assert mock_member_profile.bulk_save.call_count == 2 + assert mock_member_profile.bulk_save.call_args_list[-1][0][0] == [ + mock_profile1, + mock_profile2, + ] + @patch("apps.github.management.commands.github_update_users.MemberProfile") @patch("apps.github.management.commands.github_update_users.User") @patch("apps.github.management.commands.github_update_users.RepositoryContributor") @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 3) def test_handle_with_users_having_no_contributions( - self, mock_repository_contributor, mock_user + self, mock_repository_contributor, mock_user, mock_member_profile ): - """Test command execution when users have no contributions.""" - mock_user1 = MagicMock(id=1, title="User 1", contributions_count=0) - mock_user2 = MagicMock(id=2, title="User 2", contributions_count=0) + mock_profile1 = MagicMock(contributions_count=0) + mock_profile2 = MagicMock(contributions_count=0) + + def get_or_create_side_effect(github_user): + if github_user.id == 1: + return mock_profile1, False + if github_user.id == 2: + return mock_profile2, False + return MagicMock(), False + + mock_member_profile.objects.get_or_create.side_effect = get_or_create_side_effect + mock_user1 = MagicMock(id=1, title="User 1") + mock_user2 = MagicMock(id=2, title="User 2") mock_users_queryset = MagicMock() mock_users_queryset.count.return_value = 2 @@ -143,18 +192,26 @@ def test_handle_with_users_having_no_contributions( mock_print.assert_any_call("1 of 2 User 1") mock_print.assert_any_call("2 of 2 User 2") - assert mock_user1.contributions_count == 0 - assert mock_user2.contributions_count == 0 + assert mock_profile1.contributions_count == 0 + assert mock_profile2.contributions_count == 0 - assert mock_user.bulk_save.call_count == 1 - assert mock_user.bulk_save.call_args_list[-1][0][0] == [mock_user1, mock_user2] + assert mock_member_profile.bulk_save.call_count == 1 + assert mock_member_profile.bulk_save.call_args_list[-1][0][0] == [ + mock_profile1, + mock_profile2, + ] + @patch("apps.github.management.commands.github_update_users.MemberProfile") @patch("apps.github.management.commands.github_update_users.User") @patch("apps.github.management.commands.github_update_users.RepositoryContributor") @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 1) - def test_handle_with_single_user(self, mock_repository_contributor, mock_user): + def test_handle_with_single_user( + self, mock_repository_contributor, mock_user, mock_member_profile + ): """Test command execution with single user.""" - mock_user1 = MagicMock(id=1, title="User 1", contributions_count=0) + mock_profile1 = MagicMock(contributions_count=0) + mock_member_profile.objects.get_or_create.return_value = (mock_profile1, False) + mock_user1 = MagicMock(id=1, title="User 1") mock_users_queryset = MagicMock() mock_users_queryset.count.return_value = 1 @@ -174,16 +231,20 @@ def test_handle_with_single_user(self, mock_repository_contributor, mock_user): mock_print.assert_called_once_with("1 of 1 User 1") - assert mock_user1.contributions_count == 15 + assert mock_profile1.contributions_count == 15 - assert mock_user.bulk_save.call_count == 2 - assert mock_user.bulk_save.call_args_list[-1][0][0] == [mock_user1] + assert mock_member_profile.bulk_save.call_count == 2 + assert mock_member_profile.bulk_save.call_args_list[-1][0][0] == [mock_profile1] + @patch("apps.github.management.commands.github_update_users.MemberProfile") @patch("apps.github.management.commands.github_update_users.User") @patch("apps.github.management.commands.github_update_users.RepositoryContributor") @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 2) - def test_handle_with_empty_user_list(self, mock_repository_contributor, mock_user): + def test_handle_with_empty_user_list( + self, mock_repository_contributor, mock_user, mock_member_profile + ): """Test command execution with no users.""" + mock_member_profile.objects.get_or_create.return_value = (MagicMock(), False) mock_users_queryset = MagicMock() mock_users_queryset.count.return_value = 0 mock_users_queryset.__getitem__.return_value = [] @@ -200,16 +261,30 @@ def test_handle_with_empty_user_list(self, mock_repository_contributor, mock_use mock_print.assert_not_called() - assert mock_user.bulk_save.call_count == 1 - assert mock_user.bulk_save.call_args_list[-1][0][0] == [] + mock_member_profile.bulk_save.assert_not_called() + mock_user.bulk_save.assert_not_called() + @patch("apps.github.management.commands.github_update_users.MemberProfile") @patch("apps.github.management.commands.github_update_users.User") @patch("apps.github.management.commands.github_update_users.RepositoryContributor") @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 2) - def test_handle_with_exact_batch_size(self, mock_repository_contributor, mock_user): + def test_handle_with_exact_batch_size( + self, mock_repository_contributor, mock_user, mock_member_profile + ): """Test command execution when user count equals batch size.""" - mock_user1 = MagicMock(id=1, title="User 1", contributions_count=0) - mock_user2 = MagicMock(id=2, title="User 2", contributions_count=0) + mock_profile1 = MagicMock(contributions_count=0) + mock_profile2 = MagicMock(contributions_count=0) + + def get_or_create_side_effect(github_user): + if github_user.id == 1: + return mock_profile1, False + if github_user.id == 2: + return mock_profile2, False + return MagicMock(), False + + mock_member_profile.objects.get_or_create.side_effect = get_or_create_side_effect + mock_user1 = MagicMock(id=1, title="User 1") + mock_user2 = MagicMock(id=2, title="User 2") mock_users_queryset = MagicMock() mock_users_queryset.count.return_value = 2 @@ -232,8 +307,44 @@ def test_handle_with_exact_batch_size(self, mock_repository_contributor, mock_us mock_print.assert_any_call("1 of 2 User 1") mock_print.assert_any_call("2 of 2 User 2") - assert mock_user1.contributions_count == 10 - assert mock_user2.contributions_count == 20 + assert mock_profile1.contributions_count == 10 + assert mock_profile2.contributions_count == 20 + + assert mock_member_profile.bulk_save.call_count == 2 + assert mock_member_profile.bulk_save.call_args_list[-1][0][0] == [ + mock_profile1, + mock_profile2, + ] + + @patch("apps.github.management.commands.github_update_users.MemberProfile") + @patch("apps.github.management.commands.github_update_users.User") + @patch("apps.github.management.commands.github_update_users.RepositoryContributor") + @patch("apps.github.management.commands.github_update_users.BATCH_SIZE", 2) + def test_handle_member_profile_created( + self, mock_repository_contributor, mock_user, mock_member_profile + ): + """Test command execution when MemberProfile is newly created.""" + mock_profile = MagicMock() + mock_member_profile.objects.get_or_create.return_value = (mock_profile, True) + + mock_user1 = MagicMock( + id=1, + ) + + mock_users_queryset = MagicMock() + mock_users_queryset.count.return_value = 1 + mock_users_queryset.__getitem__.return_value = [mock_user1] + mock_user.objects.order_by.return_value = mock_users_queryset + + mock_rc_objects = MagicMock() + mock_rc_objects.exclude.return_value.values.return_value.annotate.return_value = [ + {"user_id": 1, "total_contributions": 5} + ] + mock_repository_contributor.objects = mock_rc_objects + + command = Command() + command.handle(offset=0) - assert mock_user.bulk_save.call_count == 2 - assert mock_user.bulk_save.call_args_list[-1][0][0] == [mock_user1, mock_user2] + assert mock_profile.github_user == mock_user1 + assert mock_profile.contributions_count == 5 + mock_member_profile.bulk_save.assert_called_once() diff --git a/backend/tests/apps/nest/management/commands/nest_update_badges_test.py b/backend/tests/apps/nest/management/commands/nest_update_badges_test.py index 8ec63cff2d..9f144a6308 100644 --- a/backend/tests/apps/nest/management/commands/nest_update_badges_test.py +++ b/backend/tests/apps/nest/management/commands/nest_update_badges_test.py @@ -31,13 +31,18 @@ def make_mock_former_employees(mock_former_employee): def extract_is_owasp_staff(arg): """Extract is_owasp_staff value from Q object, dict, or tuple.""" + key_to_check = "owasp_profile__is_owasp_staff" + legacy_key_to_check = "is_owasp_staff" if hasattr(arg, "children"): for key, value in arg.children: - if key == "is_owasp_staff": + if key in (key_to_check, legacy_key_to_check): return value - if isinstance(arg, dict) and "is_owasp_staff" in arg: - return arg["is_owasp_staff"] - if isinstance(arg, tuple) and len(arg) == 2 and arg[0] == "is_owasp_staff": + if isinstance(arg, dict): + if key_to_check in arg: + return arg[key_to_check] + if legacy_key_to_check in arg: + return arg[legacy_key_to_check] + if isinstance(arg, tuple) and len(arg) == 2 and arg[0] in (key_to_check, legacy_key_to_check): return arg[1] return None @@ -53,7 +58,7 @@ def get_mock_for_staff_value(value): return None def user_filter_side_effect(*args, **kwargs): - staff_value = kwargs.get("is_owasp_staff") + staff_value = kwargs.get("owasp_profile__is_owasp_staff", kwargs.get("is_owasp_staff")) if staff_value is not None: return get_mock_for_staff_value(staff_value) for arg in args: @@ -118,31 +123,32 @@ def test_sync_owasp_staff_badge( for s in ("Removed badge from 1 non-staff", "Removed badge from 1 non-employees") ) + @patch("apps.nest.management.commands.nest_update_badges.UserBadge.objects.filter") @patch("apps.nest.management.commands.nest_update_badges.Badge.objects.get_or_create") @patch("apps.nest.management.commands.nest_update_badges.User.objects.filter") - def test_badge_creation(self, mock_user_filter, mock_badge_get_or_create): + def test_badge_creation( + self, mock_user_filter, mock_badge_get_or_create, mock_user_badge_filter + ): # Set up badge creation mock mock_badge = MagicMock() mock_badge.name = OWASP_STAFF_BADGE_NAME mock_badge_get_or_create.return_value = (mock_badge, True) - # Set up empty querysets - mock_employees = MagicMock() - mock_employees.__iter__.return_value = iter([]) - mock_employees.count.return_value = 0 - mock_employees.exclude.return_value = mock_employees - mock_employees.values_list.return_value = [] - mock_employees.exclude.return_value.values_list.return_value = [] - mock_employees.exclude.return_value.distinct.return_value = mock_employees - mock_employees.exclude.return_value.distinct.return_value.values_list.return_value = [] - - mock_former_employees = MagicMock() - mock_former_employees.__iter__.return_value = iter([]) - mock_former_employees.count.return_value = 0 - mock_former_employees.values_list.return_value = [] - mock_former_employees.distinct.return_value = mock_former_employees + # Set up a detailed mock for an empty queryset + mock_empty_queryset = MagicMock() + mock_empty_queryset.count.return_value = 0 + mock_empty_queryset.exclude.return_value = mock_empty_queryset + mock_empty_queryset.distinct.return_value = mock_empty_queryset + mock_empty_queryset.values_list.return_value = [] - mock_user_filter.side_effect = [mock_employees, mock_former_employees] + # The command makes up to 4 calls to User.objects.filter for employees + # and non-employees, so we provide a mock for each. + mock_user_filter.side_effect = [ + mock_empty_queryset, + mock_empty_queryset, + mock_empty_queryset, + mock_empty_queryset, + ] out = StringIO() call_command("nest_update_badges", stdout=out) @@ -157,10 +163,15 @@ def test_badge_creation(self, mock_user_filter, mock_badge_get_or_create): ) output = out.getvalue() assert f"Created badge: {mock_badge.name}" in output + # Verify that no badges were removed since the querysets are empty + mock_user_badge_filter.assert_not_called() + @patch("apps.nest.management.commands.nest_update_badges.UserBadge.objects.filter") @patch("apps.nest.management.commands.nest_update_badges.Badge.objects.get_or_create") @patch("apps.nest.management.commands.nest_update_badges.User.objects.filter") - def test_command_idempotency(self, mock_user_filter, mock_badge_get_or_create): + def test_command_idempotency( + self, mock_user_filter, mock_badge_get_or_create, mock_user_badge_filter + ): """Test that running the command multiple times has the same effect as running it once.""" # Set up badge mock mock_badge = MagicMock() @@ -168,31 +179,15 @@ def test_command_idempotency(self, mock_user_filter, mock_badge_get_or_create): mock_badge.id = 1 mock_badge_get_or_create.return_value = (mock_badge, False) - # Set up employee mock that already has the badge - mock_employee_with_badge = MagicMock() - mock_employees = MagicMock() - mock_employees.__iter__.return_value = iter([mock_employee_with_badge]) - mock_employees.exclude.return_value = MagicMock() - mock_employees.exclude.return_value.count.return_value = 0 - mock_employees.exclude.return_value.values_list.return_value = [] - mock_employees.exclude.return_value.distinct.return_value = ( - mock_employees.exclude.return_value - ) + mock_employees_with_badge = MagicMock() + mock_employees_with_badge.exclude.return_value.count.return_value = 0 - # No former employees have the badge - mock_non_employees_filter = MagicMock() - mock_non_employees_filter.count.return_value = 0 - mock_non_employees_filter.__iter__.return_value = iter([]) - mock_non_employees_filter.values_list.return_value = [] - mock_non_employees_filter.distinct.return_value = mock_non_employees_filter + mock_non_employees = MagicMock() + mock_non_employees.distinct.return_value.count.return_value = 0 - # Configure filter side effects for two command runs - mock_user_filter.side_effect = [ - mock_employees, - mock_non_employees_filter, - mock_employees, - mock_non_employees_filter, - ] + mock_user_filter.side_effect = user_filter_side_effect_factory( + mock_employees_with_badge, mock_non_employees + ) # First run out1 = StringIO() @@ -203,17 +198,9 @@ def test_command_idempotency(self, mock_user_filter, mock_badge_get_or_create): call_command("nest_update_badges", stdout=out2) # Check both outputs contain zero-count messages - assert any( - s in out1.getvalue() for s in ("Added badge to 0 employees", "Added badge to 0 staff") - ) - assert any( - s in out1.getvalue() - for s in ("Removed badge from 0 non-employees", "Removed badge from 0 non-staff") - ) - assert any( - s in out2.getvalue() for s in ("Added badge to 0 employees", "Added badge to 0 staff") - ) - assert any( - s in out2.getvalue() - for s in ("Removed badge from 0 non-employees", "Removed badge from 0 non-staff") - ) + assert "Added badge to 0 employees" in out1.getvalue() + assert "Removed badge from 0 non-employees" in out1.getvalue() + assert "Added badge to 0 employees" in out2.getvalue() + assert "Removed badge from 0 non-employees" in out2.getvalue() + + mock_user_badge_filter.return_value.update.assert_not_called() diff --git a/backend/tests/apps/owasp/admin/member_profile_test.py b/backend/tests/apps/owasp/admin/member_profile_test.py index f5a887a39d..8d61c87ed2 100644 --- a/backend/tests/apps/owasp/admin/member_profile_test.py +++ b/backend/tests/apps/owasp/admin/member_profile_test.py @@ -12,6 +12,9 @@ def test_list_display(self): expected_fields = ( "github_user", + "is_owasp_staff", + "has_public_member_page", + "contributions_count", "owasp_slack_id", "first_contribution_at", "is_owasp_board_member",