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:
Родитель
1e7940ae7e
Коммит
dd6363030e
|
@ -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
|
||||
|
|
Загрузка…
Ссылка в новой задаче