-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Make Project -> ForeignKey -> RemoteRepository
#8259
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
Changes from all commits
2d1ff96
8160a3d
6199215
e5fccce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,19 +14,9 @@ | |
"html_url": "https://github.com/rtd/project", | ||
"modified": "2019-04-29T12:00:00Z", | ||
"name": "project", | ||
"organization": { | ||
"avatar_url": "https://avatars.githubusercontent.com/u/366329?v=4", | ||
"created": "2019-04-29T10:00:00Z", | ||
"modified": "2019-04-29T12:00:00Z", | ||
"name": "Read the Docs", | ||
"pk": 1, | ||
"slug": "readthedocs", | ||
"url": "https://github.com/readthedocs", | ||
"vcs_provider": "github" | ||
}, | ||
"pk": 1, | ||
"private": false, | ||
"project": { | ||
"projects": [{ | ||
"_links": { | ||
"_self": "https://readthedocs.org/api/v3/projects/project/", | ||
"builds": "https://readthedocs.org/api/v3/projects/project/builds/", | ||
|
@@ -58,6 +48,16 @@ | |
"versions": "https://readthedocs.org/projects/project/versions/" | ||
}, | ||
"users": [{ "username": "testuser" }] | ||
}], | ||
"remote_organization": { | ||
"avatar_url": "https://avatars.githubusercontent.com/u/366329?v=4", | ||
"created": "2019-04-29T10:00:00Z", | ||
"modified": "2019-04-29T12:00:00Z", | ||
"name": "Read the Docs", | ||
"pk": 1, | ||
"slug": "readthedocs", | ||
"url": "https://github.com/readthedocs", | ||
"vcs_provider": "github" | ||
}, | ||
"ssh_url": "[email protected]:rtd/project.git", | ||
"vcs": "git", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# Generated by Django 2.2.22 on 2021-06-14 15:42 | ||
|
||
from django.db import migrations | ||
|
||
|
||
def migrate_data(apps, schema_editor): | ||
RemoteRepository = apps.get_model('oauth', 'RemoteRepository') | ||
queryset = RemoteRepository.objects.filter(project__isnull=False).select_related('project') | ||
for rr in queryset.iterator(): | ||
rr.project.remote_repository = rr | ||
rr.save() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Took me a second to follow this logic, but I think this looks good. I suppose I would have concerns about performance here, we will have a few thousand remote repositories to step through individually. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just checked this: 22k. We should be fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I double-checked this logic and it didn't work.
That doesn't seem to keep the relation. I also tried,
I don't understand why. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meh, I found that I was just saving the wrong object. We need either:
or
However, both are producing an error that I'm not sure to understand 😞
|
||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('oauth', '0013_create_new_table_for_remote_repository_normalization'), | ||
('projects', '0076_project_remote_repository'), | ||
] | ||
|
||
operations = [ | ||
migrations.RunPython(migrate_data), | ||
migrations.RemoveField( | ||
model_name='remoterepository', | ||
name='project', | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Generated by Django 2.2.22 on 2021-06-14 15:42 | ||
|
||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('oauth', '0013_create_new_table_for_remote_repository_normalization'), | ||
('projects', '0075_change_mkdocs_name'), | ||
] | ||
|
||
operations = [ | ||
migrations.AddField( | ||
model_name='project', | ||
name='remote_repository', | ||
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='projects', to='oauth.RemoteRepository'), | ||
), | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@agjohnson this a change in the API request/response required to make sense with the new relation. It also reserves
?expand=organization
to be used forOrganization
objects.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to say we should keep
project
around for backwards compatibility, but this API should really only be internal use anyways. So yeah, agreed this seems fine.