Skip to content

Commit 965c782

Browse files
specialunderwearcrgwbrviggo-devries
authored
Optimize handling of attributes (#334)
* Optimize attribute values (#321) * Implemented faster attributes save * renamed for clarity * Renamed warning * Enable shortcut when saving child products. (#322) * Make sure to only return attributevalues that are on the actual product to avoid transferring (#331) attributesvalues from parent to child. * Improve multi db support (#335) Use the same database as the passed-in manager * The image and file fields where nolonger working. (#337) * The image and file fields where nolonger working. This PR restores that functionality. * nergens voor * fixes tests * black * Fixes cyclic import error * Fixes handling of null product image attributes When an image product attribute is `null`, the product API would throw the following exception: ``` ValueError: The 'value_image' attribute has no file associated with it. […] File "rest_framework/serializers.py", line 522, in to_representation ret[field.field_name] = field.to_representation(attribute) File "rest_framework/serializers.py", line 686, in to_representation return [ File "rest_framework/serializers.py", line 687, in <listcomp> self.child.to_representation(item) for item in iterable File "rest_framework/serializers.py", line 522, in to_representation ret[field.field_name] = field.to_representation(attribute) File "oscarapi/serializers/fields.py", line 227, in to_representation return value.value.url File "django/db/models/fields/files.py", line 66, in url self._require_file() File "django/db/models/fields/files.py", line 41, in _require_file raise ValueError( ``` This patch fixes this by checking if the image field has a value before trying to generate a URL. * Add category bulk admin api * Add tests to prove how it works * wops * Added Jenkinsfile * hehe * set to next release version * Setup local CI (#348) * Improve category upsert and add test (#352) * Doe some updating and add test * refrhes_from_db only path * Add transaction that rollsback on error * update versions --------- Co-authored-by: Craig Weber <[email protected]> Co-authored-by: Viggo de Vries <[email protected]>
1 parent f24ef46 commit 965c782

File tree

16 files changed

+1323
-90
lines changed

16 files changed

+1323
-90
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,19 @@ jobs:
1010
runs-on: ubuntu-latest
1111
steps:
1212
- name: Checkout the repository
13-
uses: actions/checkout@v2
13+
uses: actions/checkout@v4
1414
with:
1515
path: ./src
1616
- name: Setup Python 3.11
17-
uses: actions/setup-python@v2
17+
uses: actions/setup-python@v5
1818
with:
1919
python-version: '3.11'
2020
- name: Install all dependencies
2121
run: make install
2222
- name: Run all linting
2323
run: make lint
2424
- name: Upload src dir as artefact
25-
uses: actions/upload-artifact@v2
25+
uses: actions/upload-artifact@v4
2626
with:
2727
name: src
2828
path: ./src
@@ -38,12 +38,12 @@ jobs:
3838
oscar-version: ["3.2"]
3939
steps:
4040
- name: Download src dir
41-
uses: actions/download-artifact@v2
41+
uses: actions/download-artifact@v4
4242
with:
4343
name: src
4444
path: ./src
4545
- name: Setup Python 3.x
46-
uses: actions/setup-python@v2
46+
uses: actions/setup-python@v5
4747
with:
4848
python-version: ${{ matrix.python-version }}
4949
- name: Install all dependencies

‎Jenkinsfile‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/usr/bin/env groovy
2+
3+
pipeline {
4+
agent { label 'GEITENPETRA' }
5+
options { disableConcurrentBuilds() }
6+
7+
stages {
8+
stage('Build') {
9+
steps {
10+
withPythonEnv('System-CPython-3.10') {
11+
withEnv(['PIP_INDEX_URL=https://pypi.uwkm.nl/voxyan/oscar/+simple/']) {
12+
pysh "make install"
13+
}
14+
}
15+
}
16+
}
17+
stage('Lint') {
18+
steps {
19+
withPythonEnv('System-CPython-3.10') {
20+
pysh "make lint"
21+
}
22+
}
23+
}
24+
stage('Test') {
25+
steps {
26+
withPythonEnv('System-CPython-3.10') {
27+
pysh "make coverage"
28+
}
29+
}
30+
post {
31+
always {
32+
junit allowEmptyResults: true, testResults: '**/nosetests.xml'
33+
}
34+
success {
35+
step([
36+
$class: 'CoberturaPublisher',
37+
coberturaReportFile: '**/coverage.xml',
38+
])
39+
}
40+
}
41+
}
42+
}
43+
post {
44+
always {
45+
echo 'This will always run'
46+
}
47+
success {
48+
echo 'This will run only if successful'
49+
withPythonEnv('System-CPython-3.10') {
50+
echo 'This will run only if successful'
51+
pysh "version --plugin=wheel -B${env.BUILD_NUMBER} --skip-build"
52+
sh "which git"
53+
sh "git push --tags"
54+
}
55+
}
56+
failure {
57+
emailext subject: "JENKINS-NOTIFICATION: ${currentBuild.currentResult}: Job '${env.JOB_NAME} #${env.BUILD_NUMBER}'",
58+
body: '${SCRIPT, template="groovy-text.template"}',
59+
recipientProviders: [culprits(), brokenBuildSuspects(), brokenTestsSuspects()]
60+
61+
}
62+
unstable {
63+
echo 'This will run only if the run was marked as unstable'
64+
}
65+
changed {
66+
echo 'This will run only if the state of the Pipeline has changed'
67+
echo 'For example, if the Pipeline was previously failing but is now successful'
68+
}
69+
}
70+
}

‎Makefile‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ clean:
88
rm -Rf build/
99

1010
install:
11-
pip install -e .[dev]
11+
pip install -e .[dev] --upgrade --upgrade-strategy=eager --pre
1212

1313
sandbox: install
1414
python sandbox/manage.py migrate
@@ -43,15 +43,11 @@ publish_release_testpypi: build_release
4343
publish_release: build_release
4444
twine upload dist/*
4545

46-
lint.installed:
47-
pip install -e .[lint]
48-
touch $@
49-
50-
lint: lint.installed
46+
lint:
5147
black --check --exclude "migrations/*" oscarapi/
5248
pylint setup.py oscarapi/
5349

54-
black: lint.installed
50+
black:
5551
black --exclude "/migrations/" oscarapi/
5652

5753
uwsgi:

‎oscarapi/serializers/admin/product.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,15 @@ def update(self, instance, validated_data):
145145
if (
146146
self.partial
147147
): # we need to clean up all the attributes with wrong product class
148+
attribute_codes = product_class.attributes.values_list(
149+
"code", flat=True
150+
)
148151
for attribute_value in instance.attribute_values.exclude(
149152
attribute__product_class=product_class
150153
):
151154
code = attribute_value.attribute.code
152155
if (
153-
code in pclass_option_codes
156+
code in attribute_codes
154157
): # if the attribute exist also on the new product class, update the attribute
155158
attribute_value.attribute = product_class.attributes.get(
156159
code=code

‎oscarapi/serializers/fields.py‎

Lines changed: 46 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# pylint: disable=W0212, W0201, W0632
22
import logging
33
import operator
4+
import warnings
45

56
from os.path import basename, join
67
from urllib.parse import urlsplit, parse_qs
@@ -15,8 +16,10 @@
1516
from rest_framework.fields import get_attribute
1617

1718
from oscar.core.loading import get_model, get_class
19+
from oscarapi.utils.deprecations import RemovedInFutureRelease
1820

1921
from oscarapi import settings
22+
from oscarapi.utils.attributes import AttributeFieldBase, attribute_details
2023
from oscarapi.utils.loading import get_api_class
2124
from oscarapi.utils.exists import bound_unique_together_get_or_create
2225
from .exceptions import FieldError
@@ -27,7 +30,6 @@
2730
create_from_breadcrumbs = get_class("catalogue.categories", "create_from_breadcrumbs")
2831
entity_internal_value = get_api_class("serializers.hooks", "entity_internal_value")
2932
RetrieveFileMixin = get_api_class(settings.FILE_DOWNLOADER_MODULE, "RetrieveFileMixin")
30-
attribute_details = operator.itemgetter("code", "value")
3133

3234

3335
class TaxIncludedDecimalField(serializers.DecimalField):
@@ -93,7 +95,7 @@ def use_pk_only_optimization(self):
9395
return False
9496

9597

96-
class AttributeValueField(serializers.Field):
98+
class AttributeValueField(AttributeFieldBase, serializers.Field):
9799
"""
98100
This field is used to handle the value of the ProductAttributeValue model
99101
@@ -103,80 +105,56 @@ class AttributeValueField(serializers.Field):
103105
"""
104106

105107
def __init__(self, **kwargs):
108+
warnings.warn(
109+
"AttributeValueField is deprecated and will be removed in a future version of oscarapi",
110+
RemovedInFutureRelease,
111+
stacklevel=2,
112+
)
106113
# this field always needs the full object
107114
kwargs["source"] = "*"
108-
kwargs["error_messages"] = {
109-
"no_such_option": _("{code}: Option {value} does not exist."),
110-
"invalid": _("Wrong type, {error}."),
111-
"attribute_validation_error": _(
112-
"Error assigning `{value}` to {code}, {error}."
113-
),
114-
"attribute_required": _("Attribute {code} is required."),
115-
"attribute_missing": _(
116-
"No attribute exist with code={code}, "
117-
"please define it in the product_class first."
118-
),
119-
"child_without_parent": _(
120-
"Can not find attribute if product_class is empty and "
121-
"parent is empty as well, child without parent?"
122-
),
123-
}
124115
super(AttributeValueField, self).__init__(**kwargs)
125116

126117
def get_value(self, dictionary):
127118
# return all the data because this field uses everything
128119
return dictionary
129120

121+
def to_product_attribute(self, data):
122+
if "product" in data:
123+
# we need the attribute to determine the type of the value
124+
return ProductAttribute.objects.get(
125+
code=data["code"], product_class__products__id=data["product"]
126+
)
127+
elif "product_class" in data and data["product_class"] is not None:
128+
return ProductAttribute.objects.get(
129+
code=data["code"], product_class__slug=data.get("product_class")
130+
)
131+
elif "parent" in data:
132+
return ProductAttribute.objects.get(
133+
code=data["code"], product_class__products__id=data["parent"]
134+
)
135+
136+
def to_attribute_type_value(self, attribute, code, value):
137+
internal_value = super().to_attribute_type_value(attribute, code, value)
138+
if attribute.type in [
139+
attribute.IMAGE,
140+
attribute.FILE,
141+
]:
142+
image_field = ImageUrlField()
143+
image_field._context = self.context
144+
internal_value = image_field.to_internal_value(value)
145+
146+
return internal_value
147+
130148
def to_internal_value(self, data): # noqa
131149
assert "product" in data or "product_class" in data or "parent" in data
132150

133151
try:
134152
code, value = attribute_details(data)
135153
internal_value = value
136154

137-
if "product" in data:
138-
# we need the attribute to determine the type of the value
139-
attribute = ProductAttribute.objects.get(
140-
code=code, product_class__products__id=data["product"]
141-
)
142-
elif "product_class" in data and data["product_class"] is not None:
143-
attribute = ProductAttribute.objects.get(
144-
code=code, product_class__slug=data.get("product_class")
145-
)
146-
elif "parent" in data:
147-
attribute = ProductAttribute.objects.get(
148-
code=code, product_class__products__id=data["parent"]
149-
)
155+
attribute = self.to_product_attribute(data)
150156

151-
if attribute.required and value is None:
152-
self.fail("attribute_required", code=code)
153-
154-
# some of these attribute types need special processing, or their
155-
# validation will fail
156-
if attribute.type == attribute.OPTION:
157-
internal_value = attribute.option_group.options.get(option=value)
158-
elif attribute.type == attribute.MULTI_OPTION:
159-
if attribute.required and not value:
160-
self.fail("attribute_required", code=code)
161-
internal_value = attribute.option_group.options.filter(option__in=value)
162-
if len(value) != internal_value.count():
163-
non_existing = set(value) - set(
164-
internal_value.values_list("option", flat=True)
165-
)
166-
non_existing_as_error = ",".join(sorted(non_existing))
167-
self.fail("no_such_option", value=non_existing_as_error, code=code)
168-
elif attribute.type == attribute.DATE:
169-
date_field = serializers.DateField()
170-
internal_value = date_field.to_internal_value(value)
171-
elif attribute.type == attribute.DATETIME:
172-
date_field = serializers.DateTimeField()
173-
internal_value = date_field.to_internal_value(value)
174-
elif attribute.type == attribute.ENTITY:
175-
internal_value = entity_internal_value(attribute, value)
176-
elif attribute.type in [attribute.IMAGE, attribute.FILE]:
177-
image_field = ImageUrlField()
178-
image_field._context = self.context
179-
internal_value = image_field.to_internal_value(value)
157+
internal_value = self.to_attribute_type_value(attribute, code, value)
180158

181159
# the rest of the attribute types don't need special processing
182160
try:
@@ -221,10 +199,14 @@ def to_representation(self, value):
221199
return value.value.option
222200
elif obj_type == value.attribute.MULTI_OPTION:
223201
return value.value.values_list("option", flat=True)
224-
elif obj_type == value.attribute.FILE:
225-
return value.value.url
226-
elif obj_type == value.attribute.IMAGE:
227-
return value.value.url
202+
elif obj_type in [value.attribute.FILE, value.attribute.IMAGE]:
203+
if not value.value:
204+
return None
205+
url = value.value.url
206+
request = self.context.get("request", None)
207+
if request is not None:
208+
url = request.build_absolute_uri(url)
209+
return url
228210
elif obj_type == value.attribute.ENTITY:
229211
if hasattr(value.value, "json"):
230212
return value.value.json()

0 commit comments

Comments
 (0)