Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions readthedocs/oauth/migrations/0019_githubapp_add_extra_fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Generated by Django 5.2.11 on 2026-03-04 23:02

from django.db import migrations
from django.db import models
from django_safemigrate import Safe


class Migration(migrations.Migration):
safe = Safe.before_deploy()
dependencies = [
("oauth", "0018_githubapp"),
]

operations = [
migrations.AddField(
model_name="githubappinstallation",
name="all_repositories_selected",
field=models.BooleanField(
db_default=False,
default=False,
help_text="Whether the installation has access to all repositories or just some of them",
),
),
migrations.AddField(
model_name="githubappinstallation",
name="target_login",
field=models.CharField(
default="",
help_text="The account login the installation belongs to",
max_length=255,
null=True,
),
),
]
31 changes: 30 additions & 1 deletion readthedocs/oauth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,26 @@ def get_or_create_installation(
Only the installation_id is unique, the target_id and target_type could change,
but this should never happen.
"""
extra_data = extra_data or {}
installation, created = self.get_or_create(
installation_id=installation_id,
defaults={
"target_id": target_id,
"target_type": target_type,
"extra_data": extra_data or {},
"extra_data": extra_data,
},
)

# Keep the extra data about the installation up to date.
new_installation_data = extra_data.get("installation", {})
installation_has_changed = (
new_installation_data
and new_installation_data != installation.extra_data.get("installation", {})
)
if not created and installation_has_changed:
installation.extra_data = extra_data
installation.save()

# NOTE: An installation can't change its target_id or target_type.
# This should never happen, unless this assumption is wrong.
if installation.target_id != target_id or installation.target_type != target_type:
Expand Down Expand Up @@ -78,6 +90,17 @@ class GitHubAppInstallation(TimeStampedModel):
choices=GitHubAccountType,
max_length=255,
)
target_login = models.CharField(
help_text=_("The account login the installation belongs to"),
default="",
null=True,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
default="",
null=True,
null=True,
blank=True,

Remove the default="" so it uses None by default. Also add blank so we can define it from the admin.

Copy link
Member Author

Choose a reason for hiding this comment

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

All objects in production will have this field populated after the data migration.

max_length=255,
)
Comment on lines +93 to +97
Copy link
Member

Choose a reason for hiding this comment

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

What are the options for this field? user and organization only?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the login of the user/organization.

Copy link
Member

Choose a reason for hiding this comment

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

I don't follow you.

Copy link
Member Author

Choose a reason for hiding this comment

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

the username of the user/organization.

all_repositories_selected = models.BooleanField(
help_text=_("Whether the installation has access to all repositories or just some of them"),
db_default=False,
default=False,
)
extra_data = models.JSONField(
help_text=_("Extra data returned by the webhook when the installation is created"),
default=dict,
Expand Down Expand Up @@ -173,6 +196,12 @@ def delete_organization(self, organization_id: int):
target_type=self.target_type,
)

def save(self, *args, **kwargs):
installation_data = self.extra_data.get("installation", {})
self.target_login = installation_data.get("account", {}).get("login")
self.all_repositories_selected = installation_data.get("repository_selection") == "all"
super().save(*args, **kwargs)


class RemoteOrganization(TimeStampedModel):
"""
Expand Down
12 changes: 12 additions & 0 deletions readthedocs/oauth/tests/test_githubapp_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ def test_installation_created(self, sync):
"target_id": 2222,
"target_type": GitHubAccountType.USER,
},
"repository_selection": "all",
"account": {
"login": "user",
},
}
r = self.post_webhook("installation", payload)
assert r.status_code == 200
Expand All @@ -111,6 +115,8 @@ def test_installation_created(self, sync):
)
assert installation.target_id == 2222
assert installation.target_type == GitHubAccountType.USER
assert installation.all_repositories_selected
assert installation.target_login == "user"
sync.assert_called_once()

@mock.patch.object(GitHubAppService, "sync")
Expand All @@ -122,13 +128,19 @@ def test_installation_created_with_existing_installation(self, sync):
"target_id": self.installation.target_id,
"target_type": self.installation.target_type,
},
"repository_selection": "all",
"account": {
"login": "user",
},
}
r = self.post_webhook("installation", paylod)
assert r.status_code == 200
sync.assert_called_once()
self.installation.refresh_from_db()
assert self.installation.target_id == 1111
assert self.installation.target_type == GitHubAccountType.USER
assert self.installation.all_repositories_selected
assert self.installation.target_login == "user"
assert GitHubAppInstallation.objects.count() == 1

@mock.patch.object(GitHubAppService, "sync")
Expand Down