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

Add edit support for some legacyrole fields. #1943

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

jctanner
Copy link
Collaborator

@jctanner jctanner commented Oct 20, 2023

A lot of the old legacy roles had their github_user, github_repo, repository.name, namespace.name, name changed over the years. We need a way to alter those fields from the API as people change the source data.

This PR does some hacky things because the data is still all in the json fullmetadata column. In a future PR I want to migrate all the important keys out to 1st class model fields and apply proper constraints.

@github-actions github-actions bot added backport-4.2 This PR should be backported to stable-4.2 (1.2) backport-4.4 This PR should be backported to stable-4.4 (2.1) backport-4.5 This PR should be backported to stable-4.5 (2.2) backport-4.6 This PR should be backported to stable-4.6 (2.3) backport-4.7 This PR should be backported to stable-4.7 (2.4) backport-4.8 This PR should be backported to stable-4.8 (2.4) labels Oct 20, 2023
@jctanner jctanner removed backport-4.2 This PR should be backported to stable-4.2 (1.2) backport-4.4 This PR should be backported to stable-4.4 (2.1) backport-4.5 This PR should be backported to stable-4.5 (2.2) backport-4.6 This PR should be backported to stable-4.6 (2.3) backport-4.7 This PR should be backported to stable-4.7 (2.4) backport-4.8 This PR should be backported to stable-4.8 (2.4) labels Oct 21, 2023
No-Issue

Signed-off-by: James Tanner <[email protected]>
No-Issue

Signed-off-by: James Tanner <[email protected]>
No-Issue

Signed-off-by: James Tanner <[email protected]>
@@ -336,6 +342,40 @@ def get_download_count(self, obj):
return 0


class LegacyRoleRepositoryUpdateSerializer(serializers.Serializer):
name = serializers.CharField(required=False, allow_blank=False, max_length=50)
original_name = serializers.CharField(required=False, allow_blank=False, max_length=50)
Copy link
Contributor

@jerabekjiri jerabekjiri Oct 31, 2023

Choose a reason for hiding this comment

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

I'm able to edit original_name in the repository, is that intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good question. I think longterm, no you shouldn't be able to. For the short term though, we need to be able to set that based on the old api data.

@jerabekjiri
Copy link
Contributor

jerabekjiri commented Oct 31, 2023

EDIT:
with cURL it works fine

curl -X PUT \                                                                                                                        
-H "Content-Type: application/json" \
-d '{"username": "test" }' \
--user "admin:admin" \
http://localhost:5001/api/v1/roles/1/

{"username":["Unexpected field."]}

Updating some values like username or id in browsable API throws
Screenshot from 2023-10-31 15-22-26

Copy link
Contributor

@jerabekjiri jerabekjiri left a comment

Choose a reason for hiding this comment

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

LGTM

but question: we don't allow updating tags or description, AFAIK that happens when a user creates the role and to update, he has to create a new version with updated desc and tags?

@jctanner
Copy link
Collaborator Author

jctanner commented Oct 31, 2023

@jerabekjiri yep, description and tags will come from the meta/main.yml at [re-]import time. The editable fields in this PR were selected because they directly affect imports and installs.

@jctanner jctanner merged commit d40c8b5 into ansible:master Oct 31, 2023
19 of 21 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.

2 participants