allow custom licenses to be created and updated per version (#18368)

* allow custom licenses to be created and updated per version

* Add valiation check for both fields being provided
This commit is contained in:
Andrew Williamson 2021-11-23 17:50:06 +00:00 коммит произвёл GitHub
Родитель 1e7940ae7e
Коммит dd6363030e
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
4 изменённых файлов: 301 добавлений и 9 удалений

Просмотреть файл

@ -472,6 +472,9 @@ This endpoint allows a submission of an upload to an existing add-on to create a
production environments. See :ref:`the older signing API <upload-version>` for
how to submit new add-ons/versions on AMO today.
.. note::
This API requires :doc:`authentication <auth>`.
.. http:post:: /api/v5/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/
.. _version-create-request:
@ -482,7 +485,9 @@ This endpoint allows a submission of an upload to an existing add-on to create a
where min/max versions from the manifest, or defaults, will be used. See :ref:`examples <version-compatibility-examples>`.
:<json string compatibility[app_name].max: Maximum version of the corresponding app the version is compatible with. Should only be enforced by clients if ``is_strict_compatibility_enabled`` is ``true``.
:<json string compatibility[app_name].min: Minimum version of the corresponding app the version is compatible with.
:<json int license: The license id.
:<json int license: The id of a non-custom license. The license must match the add-on type. Either provide ``license`` or ``custom_license``, not both.
:<json object|null custom_license.name: The name of the license (See :ref:`translated fields <api-overview-translations>`). Custom licenses are not supported for themes.
:<json object|null custom_license.text: The text of the license (See :ref:`translated fields <api-overview-translations>`). Custom licenses are not supported for themes.
:<json object|null release_notes: The release notes for this version (See :ref:`translated fields <api-overview-translations>`).
:<json string upload: The uuid for the xpi upload to create this version with.
@ -552,6 +557,9 @@ This endpoint allows the metadata for an existing version to be edited.
production environments. The only supported method of editing metadata today is
via developer hub.
.. note::
This API requires :doc:`authentication <auth>`.
.. http:patch:: /api/v5/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/
.. _version-edit-request:
@ -559,7 +567,9 @@ This endpoint allows the metadata for an existing version to be edited.
:<json object|array compatibility: Either an object detailing which :ref:`applications <addon-detail-application>` and versions the version is compatible with; or an array of :ref:`applications <addon-detail-application>`, where default min/max versions will be used if not already defined. See :ref:`examples <version-compatibility-examples>`.
:<json string compatibility[app_name].max: Maximum version of the corresponding app the version is compatible with. Should only be enforced by clients if ``is_strict_compatibility_enabled`` is ``true``.
:<json string compatibility[app_name].min: Minimum version of the corresponding app the version is compatible with.
:<json int license: The license id.
:<json int license: The id of a non-custom license. The license must match the add-on type. Either provide ``license`` or ``custom_license``, not both.
:<json object|null custom_license.name: The name of the license (See :ref:`translated fields <api-overview-translations>`). Custom licenses are not supported for themes.
:<json object|null custom_license.text: The text of the license (See :ref:`translated fields <api-overview-translations>`). Custom licenses are not supported for themes.
:<json object|null release_notes: The release notes for this version (See :ref:`translated fields <api-overview-translations>`).
@ -576,6 +586,9 @@ This endpoint is for uploading an addon file, to then be submitted to create a n
production environments. See :ref:`the older signing API <upload-version>` for
how to submit new add-ons/versions on AMO today.
.. note::
This API requires :doc:`authentication <auth>`.
.. http:post:: /api/v5/addons/upload/
.. _upload-create-request:

Просмотреть файл

@ -428,6 +428,7 @@ These are `v5` specific changes - `v4` changes apply also.
* 2021-09-23: flattened ``files`` in version detail from an array to a single ``file``. https://github.com/mozilla/addons-server/issues/17839
* 2021-09-30: removed ``is_webextension`` from file objects (all addons have been webextensions for a while now) in all endpoints. https://github.com/mozilla/addons-server/issues/17658
* 2021-11-18: added docs for the under-development addon submission & edit apis.
* 2021-11-25: added `custom_license` to version create/edit endpoints to allow non-predefined licenses to be created and updated. https://github.com/mozilla/addons-server/issues/18034
.. _`#11380`: https://github.com/mozilla/addons-server/issues/11380/
.. _`#11379`: https://github.com/mozilla/addons-server/issues/11379/

Просмотреть файл

@ -180,6 +180,9 @@ class LicenseNameSerializerField(serializers.Field):
# get_attribute(), we just have to return the data at this point.
return obj
def to_internal_value(self, value):
return self.custom_translation_field.to_internal_value(value)
class ESLicenseNameSerializerField(LicenseNameSerializerField):
"""Like LicenseNameSerializerField, but uses the data from ES to avoid
@ -205,6 +208,8 @@ class LicenseSerializer(serializers.ModelSerializer):
class Meta:
model = License
fields = ('id', 'is_custom', 'name', 'text', 'url')
writeable_fields = ('name', 'text')
read_only_fields = tuple(set(fields) - set(writeable_fields))
def get_is_custom(self, obj):
return not bool(obj.builtin)
@ -231,6 +236,11 @@ class LicenseSerializer(serializers.ModelSerializer):
data.pop('is_custom', None)
return data
def validate(self, data):
if self.instance and not self.get_is_custom(self.instance):
raise exceptions.ValidationError('Built in licenses can not be updated.')
return data
class CompactLicenseSerializer(LicenseSerializer):
class Meta:
@ -373,10 +383,15 @@ class VersionSerializer(SimpleVersionSerializer):
)
license = SplitField(
serializers.PrimaryKeyRelatedField(
queryset=License.objects.builtins(), required=False
queryset=License.objects.exclude(builtin=License.OTHER), required=False
),
LicenseSerializer(),
)
custom_license = LicenseSerializer(
write_only=True,
required=False,
source='license',
)
upload = serializers.SlugRelatedField(
slug_field='uuid', queryset=FileUpload.objects.all(), write_only=True
)
@ -387,6 +402,7 @@ class VersionSerializer(SimpleVersionSerializer):
'id',
'channel',
'compatibility',
'custom_license',
'edit_url',
'file',
'is_strict_compatibility_enabled',
@ -398,6 +414,7 @@ class VersionSerializer(SimpleVersionSerializer):
)
writeable_fields = (
'compatibility',
'custom_license',
'license',
'release_notes',
'upload',
@ -449,7 +466,12 @@ class VersionSerializer(SimpleVersionSerializer):
# We test for new addons in AddonSerailizer.validate instead
if channel == amo.RELEASE_CHANNEL_LISTED and not data.get('license'):
raise exceptions.ValidationError(
{'license': 'This field is required for listed versions.'},
{
'license': (
'This field, or custom_license, is required for listed '
'versions.'
)
},
code='required',
)
@ -470,8 +492,41 @@ class VersionSerializer(SimpleVersionSerializer):
f'version: {missing_addon_metadata}.',
code='required',
)
addon_type = self.parsed_data['type']
else:
data.pop('upload', None) # upload can only be set during create
addon_type = self.addon.type
# We have to check the raw request data because data from both fields will be
# under `license` at this point.
if (
(request := self.context.get('request'))
and 'license' in request.data
and 'custom_license' in request.data
):
raise exceptions.ValidationError(
'Both `license` and `custom_license` cannot be provided together.'
)
if (
'license' in data
and isinstance(data['license'], License)
and data['license'].creative_commons
== (addon_type != amo.ADDON_STATICTHEME)
):
raise exceptions.ValidationError(
{'license': 'Wrong addon type for this license.'},
code='required',
)
if (
'license' in data
and isinstance(data['license'], dict)
and addon_type == amo.ADDON_STATICTHEME
):
raise exceptions.ValidationError(
{'custom_license': 'Custom licenses are not supported for themes.'},
)
return data
def create(self, validated_data):
@ -480,7 +535,7 @@ class VersionSerializer(SimpleVersionSerializer):
**self.parsed_data,
**validated_data,
}
if 'license' in validated_data:
if isinstance(validated_data.get('license'), License):
parsed_and_validated_data['license_id'] = validated_data['license'].id
version = Version.from_upload(
upload=upload,
@ -489,6 +544,11 @@ class VersionSerializer(SimpleVersionSerializer):
compatibility=validated_data.get('compatible_apps'),
parsed_data=parsed_and_validated_data,
)
if isinstance(validated_data.get('license'), dict):
# If we got a custom license lets create it and assign it to the version.
version.update(
license=self.fields['custom_license'].create(validated_data['license'])
)
upload.update(addon=version.addon)
if (
self.addon
@ -500,9 +560,24 @@ class VersionSerializer(SimpleVersionSerializer):
return version
def update(self, instance, validated_data):
custom_license = (
validated_data.pop('license')
if isinstance(validated_data.get('license'), dict)
else None
)
instance = super().update(instance, validated_data)
if 'compatible_apps' in validated_data:
instance.set_compatible_apps(validated_data['compatible_apps'])
if custom_license:
if (
existing := getattr(instance, 'license', None)
) and existing.builtin == License.OTHER:
self.fields['custom_license'].update(existing, custom_license)
else:
instance.update(
license=self.fields['custom_license'].create(custom_license)
)
return instance

Просмотреть файл

@ -743,7 +743,10 @@ class TestAddonViewSetCreate(UploadMixin, TestCase):
self.url,
data={
'categories': {'firefox': ['bookmarks']},
'version': {'upload': self.upload.uuid, 'license': self.license.id},
'version': {
'upload': self.upload.uuid,
'license': self.license.id,
},
},
)
assert response.status_code == 201, response.content
@ -769,7 +772,11 @@ class TestAddonViewSetCreate(UploadMixin, TestCase):
)
assert response.status_code == 400, response.content
assert response.data == {
'version': {'license': ['This field is required for listed versions.']},
'version': {
'license': [
'This field, or custom_license, is required for listed versions.'
]
},
}
# If the license is set we'll get further validation errors from addon
@ -785,7 +792,10 @@ class TestAddonViewSetCreate(UploadMixin, TestCase):
self.url,
data={
'summary': {'en-US': 'replacement summary'},
'version': {'upload': self.upload.uuid, 'license': self.license.id},
'version': {
'upload': self.upload.uuid,
'license': self.license.id,
},
},
)
assert response.status_code == 400, response.content
@ -1646,7 +1656,9 @@ class TestVersionViewSetCreate(UploadMixin, TestCase):
)
assert response.status_code == 400, response.content
assert response.data == {
'license': ['This field is required for listed versions.'],
'license': [
'This field, or custom_license, is required for listed versions.'
],
}
# If the license is set we'll get further validation errors from about the addon
@ -1776,6 +1788,59 @@ class TestVersionViewSetCreate(UploadMixin, TestCase):
assert 'Version 0.0.1 matches ' in str(response.data['non_field_errors'])
assert self.addon.reload().versions.count() == 1
def test_custom_license(self):
self.upload.update(automated_signing=False)
self.addon.current_version.file.update(status=amo.STATUS_DISABLED)
self.addon.update_status()
assert self.addon.status == amo.STATUS_NULL
license_data = {
'name': {'en-US': 'my custom license name'},
'text': {'en-US': 'my custom license text'},
}
response = self.client.post(
self.url,
data={**self.minimal_data, 'custom_license': license_data},
)
assert response.status_code == 201, response.content
data = response.data
self.addon.reload()
assert self.addon.versions.count() == 2
version = self.addon.find_latest_version(channel=None)
assert version.channel == amo.RELEASE_CHANNEL_LISTED
assert self.addon.status == amo.STATUS_NOMINATED
new_license = License.objects.latest('created')
assert version.license == new_license
assert data['license'] == {
'id': new_license.id,
'name': license_data['name'],
'text': license_data['text'],
'is_custom': True,
'url': 'http://testserver' + version.license_url(),
}
def test_cannot_supply_both_custom_and_license_id(self):
license_data = {
'name': {'en-US': 'custom license name'},
'text': {'en-US': 'custom license text'},
}
response = self.client.post(
self.url,
data={
**self.minimal_data,
'license': self.license.id,
'custom_license': license_data,
},
)
assert response.status_code == 400, response.content
assert response.data == {
'non_field_errors': [
'Both `license` and `custom_license` cannot be provided together.'
]
}
class TestVersionViewSetCreateJWTAuth(TestVersionViewSetCreate):
client_class = JWTAPITestClient
@ -1984,6 +2049,144 @@ class TestVersionViewSetUpdate(UploadMixin, TestCase):
)
assert response.data == {'compatibility': ['Unknown app version specified']}
def test_custom_license(self):
# First assume no license - edge case because we enforce a license for listed
# versions, but possible.
self.version.update(license=None)
license_data = {
'name': {'en-US': 'custom license name'},
'text': {'en-US': 'custom license text'},
}
response = self.client.patch(
self.url,
data={'custom_license': license_data},
)
assert response.status_code == 200, response.content
data = response.data
self.version.reload()
new_license = License.objects.latest('created')
assert self.version.license == new_license
assert data['license'] == {
'id': new_license.id,
'name': license_data['name'],
'text': license_data['text'],
'is_custom': True,
'url': 'http://testserver' + self.version.license_url(),
}
# And then check we can update an existing custom license
num_licenses = License.objects.count()
response = self.client.patch(
self.url,
data={'custom_license': {'name': {'en-US': 'neú name'}}},
)
assert response.status_code == 200, response.content
data = response.data
self.version.reload()
new_license.reload()
assert self.version.license == new_license
assert data['license'] == {
'id': new_license.id,
'name': {'en-US': 'neú name'},
'text': license_data['text'], # no change
'is_custom': True,
'url': 'http://testserver' + self.version.license_url(),
}
assert new_license.name == 'neú name'
assert License.objects.count() == num_licenses
def test_custom_license_from_builtin(self):
assert self.version.license.builtin != License.OTHER
builtin_license = self.version.license
license_data = {
'name': {'en-US': 'custom license name'},
'text': {'en-US': 'custom license text'},
}
response = self.client.patch(
self.url,
data={'custom_license': license_data},
)
assert response.status_code == 200, response.content
data = response.data
self.version.reload()
new_license = License.objects.latest('created')
assert self.version.license == new_license
assert new_license != builtin_license
assert data['license'] == {
'id': new_license.id,
'name': license_data['name'],
'text': license_data['text'],
'is_custom': True,
'url': 'http://testserver' + self.version.license_url(),
}
# and check we can change back to a builtin from a custom license
response = self.client.patch(
self.url,
data={'license': builtin_license.id},
)
assert response.status_code == 200, response.content
data = response.data
self.version.reload()
assert self.version.license == builtin_license
assert data['license']['id'] == builtin_license.id
assert data['license']['name']['en-US'] == builtin_license.name
assert data['license']['is_custom'] is False
assert data['license']['url'] == builtin_license.url
def test_no_custom_license_for_themes(self):
self.addon.update(type=amo.ADDON_STATICTHEME)
license_data = {
'name': {'en-US': 'custom license name'},
'text': {'en-US': 'custom license text'},
}
response = self.client.patch(
self.url,
data={'custom_license': license_data},
)
assert response.status_code == 400, response.content
assert response.data == {
'custom_license': ['Custom licenses are not supported for themes.']
}
def test_license_type_matches_addon_type(self):
self.addon.update(type=amo.ADDON_STATICTHEME)
response = self.client.patch(
self.url,
data={'license': self.version.license.id},
)
assert response.status_code == 400, response.content
assert response.data == {'license': ['Wrong addon type for this license.']}
self.addon.update(type=amo.ADDON_EXTENSION)
self.version.license.update(creative_commons=True)
response = self.client.patch(
self.url,
data={'license': self.version.license.id},
)
assert response.status_code == 400, response.content
assert response.data == {'license': ['Wrong addon type for this license.']}
def test_cannot_supply_both_custom_and_license_id(self):
license_data = {
'name': {'en-US': 'custom license name'},
'text': {'en-US': 'custom license text'},
}
response = self.client.patch(
self.url,
data={'license': self.version.license.id, 'custom_license': license_data},
)
assert response.status_code == 400, response.content
assert response.data == {
'non_field_errors': [
'Both `license` and `custom_license` cannot be provided together.'
]
}
class TestVersionViewSetUpdateJWTAuth(TestVersionViewSetUpdate):
client_class = JWTAPITestClient