From 870ed6ae3044ab7b72b27e01a69d7945db2dc71f Mon Sep 17 00:00:00 2001 From: stefanmajoor Date: Fri, 17 Sep 2021 13:42:55 +0200 Subject: [PATCH 1/4] add support fo nullable foreign keys in CSVexport plugin --- binder/plugins/views/csvexport.py | 7 ++++++- tests/plugins/test_csvexport.py | 30 +++++++++++++++++++++++++++--- tests/testapp/models/__init__.py | 2 +- tests/testapp/models/picture.py | 13 +++++++++++++ tests/testapp/views/__init__.py | 2 +- tests/testapp/views/picture.py | 7 +++++-- 6 files changed, 53 insertions(+), 8 deletions(-) diff --git a/binder/plugins/views/csvexport.py b/binder/plugins/views/csvexport.py index 6e66d2ff..294bd369 100644 --- a/binder/plugins/views/csvexport.py +++ b/binder/plugins/views/csvexport.py @@ -284,7 +284,12 @@ def get_datum(data, key, prefix=''): else: # Assume that we have a mapping now fk_ids = data[head_key] - if type(fk_ids) != list: + + if fk_ids is None: + # This case happens if we have a nullable foreign key that is null. Treat this as a many + # to one relation with no values. + fk_ids = [] + elif type(fk_ids) != list: fk_ids = [fk_ids] # if head_key not in key_mapping: diff --git a/tests/plugins/test_csvexport.py b/tests/plugins/test_csvexport.py index 52c0b406..d2d4d010 100644 --- a/tests/plugins/test_csvexport.py +++ b/tests/plugins/test_csvexport.py @@ -7,7 +7,7 @@ from django.core.files import File from django.contrib.auth.models import User -from ..testapp.models import Picture, Animal +from ..testapp.models import Picture, Animal, PictureBook import csv import openpyxl @@ -31,8 +31,10 @@ def setUp(self): self.pictures = [] + picture_book = PictureBook.objects.create(name='Holiday 2012') + for i in range(3): - picture = Picture(animal=animal) + picture = Picture(animal=animal, picture_book=picture_book) file = CsvExportTest.temp_imagefile(50, 50, 'jpeg') picture.file.save('picture.jpg', File(file), save=False) picture.original_file.save('picture_copy.jpg', File(file), save=False) @@ -48,6 +50,7 @@ def setUp(self): def test_csv_download(self): response = self.client.get('/picture/download_csv/') + print(response.content) self.assertEqual(200, response.status_code) response_data = csv.reader(io.StringIO(response.content.decode("utf-8"))) @@ -123,4 +126,25 @@ def test_context_aware_download_xlsx(self): self.assertEqual(list(_values[2]), [self.pictures[1].id, str(self.pictures[1].animal_id), (self.pictures[1].id ** 2)]) self.assertEqual(list(_values[3]), - [self.pictures[2].id, str(self.pictures[2].animal_id), (self.pictures[2].id ** 2)]) \ No newline at end of file + [self.pictures[2].id, str(self.pictures[2].animal_id), (self.pictures[2].id ** 2)]) + + def test_none_foreign_key(self): + """ + If we have a relation that we have to include which is nullable, and we have a foreign key, we do not want the + csv export to crash. I.e. we also want for a picture to export the picture book name, even though not all pioctures + belong to a picture book + :return: + """ + self.pictures[0].picture_book = None + self.pictures[0].save() + + response = self.client.get('/picture/download/') + self.assertEqual(200, response.status_code) + response_data = csv.reader(io.StringIO(response.content.decode("utf-8"))) + + data = list(response_data) + content = data[1:] + + picture_books = [c[-1] for c in content] + + self.assertEqual(['', 'Holiday 2012', 'Holiday 2012'], picture_books) \ No newline at end of file diff --git a/tests/testapp/models/__init__.py b/tests/testapp/models/__init__.py index ed41bbe5..7ccddcf7 100644 --- a/tests/testapp/models/__init__.py +++ b/tests/testapp/models/__init__.py @@ -9,7 +9,7 @@ from .gate import Gate from .nickname import Nickname, NullableNickname from .lion import Lion -from .picture import Picture +from .picture import Picture, PictureBook from .zoo import Zoo from .zoo_employee import ZooEmployee from .city import City, CityState, PermanentCity diff --git a/tests/testapp/models/picture.py b/tests/testapp/models/picture.py index ffbe59bb..862bfb3e 100644 --- a/tests/testapp/models/picture.py +++ b/tests/testapp/models/picture.py @@ -13,6 +13,18 @@ def delete_files(sender, instance=None, **kwargs): except Exception: pass + +class PictureBook(BinderModel): + """ + Sometimes customers like to commemorate their visit to the zoo. Of course there are always some shitty pictures that + we do not want in a picture album + + """ + + name = models.TextField() + + + # At the website of the zoo there are some pictures of animals. This model links the picture to an animal. # # A picture has two files, the original uploaded file, and the modified file. This model is used for testing the @@ -21,6 +33,7 @@ class Picture(BinderModel): animal = models.ForeignKey('Animal', on_delete=models.CASCADE, related_name='picture') file = models.ImageField(upload_to='floor-plans') original_file = models.ImageField(upload_to='floor-plans') + picture_book = models.ForeignKey('PictureBook', on_delete=models.CASCADE, null=True, blank=True) def __str__(self): return 'picture %d: (Picture for animal %s)' % (self.pk or 0, self.animal.name) diff --git a/tests/testapp/views/__init__.py b/tests/testapp/views/__init__.py index 7dd1f4f0..84989ceb 100644 --- a/tests/testapp/views/__init__.py +++ b/tests/testapp/views/__init__.py @@ -15,7 +15,7 @@ from .gate import GateView from .lion import LionView from .nickname import NicknameView -from .picture import PictureView +from .picture import PictureView, PictureBookView from .user import UserView from .zoo import ZooView from .zoo_employee import ZooEmployeeView diff --git a/tests/testapp/views/picture.py b/tests/testapp/views/picture.py index 7dadfe99..0ba35f99 100644 --- a/tests/testapp/views/picture.py +++ b/tests/testapp/views/picture.py @@ -3,16 +3,19 @@ from binder.views import ModelView from binder.plugins.views import ImageView, CsvExportView -from ..models import Picture +from ..models import Picture, PictureBook +class PictureBookView(ModelView): + model = PictureBook class PictureView(ModelView, ImageView, CsvExportView): model = Picture file_fields = ['file', 'original_file'] - csv_settings = CsvExportView.CsvExportSettings(['animal'], [ + csv_settings = CsvExportView.CsvExportSettings(['animal', 'picture_book'], [ ('id', 'picture identifier'), ('animal.id', 'animal identifier'), ('id', 'squared picture identifier', lambda id, row, mapping: id**2), + ('picture_book.name', 'Picturebook name') ]) @list_route(name='download_csv', methods=['GET']) From d02e7ffe2d9a0211888b76e0baa44f13e47e2601 Mon Sep 17 00:00:00 2001 From: stefanmajoor Date: Mon, 20 Sep 2021 11:04:47 +0200 Subject: [PATCH 2/4] add support for sets, list and dictionaries in csvexport --- binder/plugins/views/csvexport.py | 5 +++ tests/plugins/test_csvexport.py | 57 +++++++++++++++++++++---------- tests/testapp/views/picture.py | 7 +++- 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/binder/plugins/views/csvexport.py b/binder/plugins/views/csvexport.py index 294bd369..8d4cbf36 100644 --- a/binder/plugins/views/csvexport.py +++ b/binder/plugins/views/csvexport.py @@ -116,6 +116,11 @@ def set_columns(self, columns: List[str]): def add_row(self, values: List[str]): for (column_id, value) in enumerate(values): + + # The excel library doesn't implicitely cast this to string, so do it explicitely to prevent + # the export from crashing + if type(value) in [dict, list, set]: + value = str(value) self.sheet.cell(column=column_id + 1, row=self._row_number + 1, value=value) self._row_number += 1 diff --git a/tests/plugins/test_csvexport.py b/tests/plugins/test_csvexport.py index d2d4d010..8a069a03 100644 --- a/tests/plugins/test_csvexport.py +++ b/tests/plugins/test_csvexport.py @@ -50,19 +50,18 @@ def setUp(self): def test_csv_download(self): response = self.client.get('/picture/download_csv/') - print(response.content) self.assertEqual(200, response.status_code) response_data = csv.reader(io.StringIO(response.content.decode("utf-8"))) data = list(response_data) # First line needs to be the header - self.assertEqual(data[0], ['picture identifier', 'animal identifier', 'squared picture identifier']) + self.assertEqual(data[0][:3], ['picture identifier', 'animal identifier', 'squared picture identifier']) # All other data needs to be ordered using the default ordering (by id, asc) - self.assertEqual(data[1], [str(self.pictures[0].id), str(self.pictures[0].animal_id), str(self.pictures[0].id ** 2)]) - self.assertEqual(data[2], [str(self.pictures[1].id), str(self.pictures[1].animal_id), str(self.pictures[1].id ** 2)]) - self.assertEqual(data[3], [str(self.pictures[2].id), str(self.pictures[2].animal_id), str(self.pictures[2].id ** 2)]) + self.assertEqual(data[1][:3], [str(self.pictures[0].id), str(self.pictures[0].animal_id), str(self.pictures[0].id ** 2)]) + self.assertEqual(data[2][:3], [str(self.pictures[1].id), str(self.pictures[1].animal_id), str(self.pictures[1].id ** 2)]) + self.assertEqual(data[3][:3], [str(self.pictures[2].id), str(self.pictures[2].animal_id), str(self.pictures[2].id ** 2)]) def test_excel_download(self): @@ -77,14 +76,14 @@ def test_excel_download(self): _values = list(sheet.values) # First line needs to be the header - self.assertEqual(list(_values[0]), ['picture identifier', 'animal identifier', 'squared picture identifier']) + self.assertEqual(list(_values[0][:3]), ['picture identifier', 'animal identifier', 'squared picture identifier']) # All other data needs to be ordered using the default ordering (by id, asc) - self.assertEqual(list(_values[1]), + self.assertEqual(list(_values[1][:3]), [self.pictures[0].id, str(self.pictures[0].animal_id), (self.pictures[0].id ** 2)]) - self.assertEqual(list(_values[2]), + self.assertEqual(list(_values[2][:3]), [self.pictures[1].id, str(self.pictures[1].animal_id), (self.pictures[1].id ** 2)]) - self.assertEqual(list(_values[3]), + self.assertEqual(list(_values[3][:3]), [self.pictures[2].id, str(self.pictures[2].animal_id), (self.pictures[2].id ** 2)]) @@ -98,12 +97,12 @@ def test_context_aware_downloader_default_csv(self): data = list(response_data) # First line needs to be the header - self.assertEqual(data[0], ['picture identifier', 'animal identifier', 'squared picture identifier']) + self.assertEqual(data[0][:3], ['picture identifier', 'animal identifier', 'squared picture identifier']) # All other data needs to be ordered using the default ordering (by id, asc) - self.assertEqual(data[1], [str(self.pictures[0].id), str(self.pictures[0].animal_id), str(self.pictures[0].id ** 2)]) - self.assertEqual(data[2], [str(self.pictures[1].id), str(self.pictures[1].animal_id), str(self.pictures[1].id ** 2)]) - self.assertEqual(data[3], [str(self.pictures[2].id), str(self.pictures[2].animal_id), str(self.pictures[2].id ** 2)]) + self.assertEqual(data[1][:3], [str(self.pictures[0].id), str(self.pictures[0].animal_id), str(self.pictures[0].id ** 2)]) + self.assertEqual(data[2][:3], [str(self.pictures[1].id), str(self.pictures[1].animal_id), str(self.pictures[1].id ** 2)]) + self.assertEqual(data[3][:3], [str(self.pictures[2].id), str(self.pictures[2].animal_id), str(self.pictures[2].id ** 2)]) def test_context_aware_download_xlsx(self): response = self.client.get('/picture/download/?response_type=xlsx') @@ -118,14 +117,14 @@ def test_context_aware_download_xlsx(self): _values = list(sheet.values) # First line needs to be the header - self.assertEqual(list(_values[0]), ['picture identifier', 'animal identifier', 'squared picture identifier']) + self.assertEqual(list(_values[0][:3]), ['picture identifier', 'animal identifier', 'squared picture identifier']) # All other data needs to be ordered using the default ordering (by id, asc) - self.assertEqual(list(_values[1]), + self.assertEqual(list(_values[1][:3]), [self.pictures[0].id, str(self.pictures[0].animal_id), (self.pictures[0].id ** 2)]) - self.assertEqual(list(_values[2]), + self.assertEqual(list(_values[2][:3]), [self.pictures[1].id, str(self.pictures[1].animal_id), (self.pictures[1].id ** 2)]) - self.assertEqual(list(_values[3]), + self.assertEqual(list(_values[3][:3]), [self.pictures[2].id, str(self.pictures[2].animal_id), (self.pictures[2].id ** 2)]) def test_none_foreign_key(self): @@ -147,4 +146,26 @@ def test_none_foreign_key(self): picture_books = [c[-1] for c in content] - self.assertEqual(['', 'Holiday 2012', 'Holiday 2012'], picture_books) \ No newline at end of file + self.assertEqual(['', 'Holiday 2012', 'Holiday 2012'], picture_books) + + def test_convert_list_dictionary_set_excel(self): + """ + In the first implementation there was a bug where returning a list or dictionary in the csv export worked for + csv, but not for excel. This checks if we can retun list and dictionaries + """ + response = self.client.get('/picture/download_excel/') + + with NamedTemporaryFile(suffix='.xlsx') as tmp: + tmp.write(response.content) + + wb = openpyxl.load_workbook(tmp.name) + sheet = wb._sheets[1] + + _values = list(sheet.values) + + # Check for the correct titles + self.assertEqual(('dictionary_example', 'list_example', 'set_example'), (_values[0][4:7])) + + id = str(self.pictures[0].id) + # And the content of the first row + self.assertEqual(("{'id': " +id + "}", "[" +id + "]", "{" +id + "}"), (_values[1][4:7])) diff --git a/tests/testapp/views/picture.py b/tests/testapp/views/picture.py index 0ba35f99..9c7ffe36 100644 --- a/tests/testapp/views/picture.py +++ b/tests/testapp/views/picture.py @@ -5,9 +5,11 @@ from ..models import Picture, PictureBook + class PictureBookView(ModelView): model = PictureBook + class PictureView(ModelView, ImageView, CsvExportView): model = Picture file_fields = ['file', 'original_file'] @@ -15,7 +17,10 @@ class PictureView(ModelView, ImageView, CsvExportView): ('id', 'picture identifier'), ('animal.id', 'animal identifier'), ('id', 'squared picture identifier', lambda id, row, mapping: id**2), - ('picture_book.name', 'Picturebook name') + ('picture_book.name', 'Picturebook name'), + ('id', 'dictionary_example', lambda id, row, mapping: {'id': id}), + ('id', 'list_example', lambda id, row, mapping: [id]), + ('id', 'set_example', lambda id, row, mapping: {id}) ]) @list_route(name='download_csv', methods=['GET']) From 6d0c6222d0402e5244d036cd4bf34a1b2ee2b51c Mon Sep 17 00:00:00 2001 From: Daan van der Kallen Date: Mon, 27 Sep 2021 14:22:59 +0200 Subject: [PATCH 3/4] Remove broken cache --- .github/workflows/ci.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d2f4423..403dc1b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,18 +46,10 @@ jobs: with: python-version: ${{ matrix.python-version }} - - name: Retrieve cached venv - uses: actions/cache@v1 - id: cache-venv - with: - path: ./.venv/ - key: ${{ runner.os }}-${{ matrix.python-version }}-${{ matrix.django-version }}-venv-${{ hashFiles('ci-requirements.txt') }} - - name: Install requirements run: | python -m venv .venv .venv/bin/pip install -qr ci-requirements.txt django==${{ matrix.django-version }} - if: steps.cache-venv.outputs.cache-hit != 'true' - name: Run linting run: .venv/bin/flake8 binder From 24e66f5ef73487340ff1b3a2cdada5cc57c1d529 Mon Sep 17 00:00:00 2001 From: Daan van der Kallen Date: Mon, 27 Sep 2021 14:31:07 +0200 Subject: [PATCH 4/4] Remove trailing whitespace --- binder/plugins/views/csvexport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/binder/plugins/views/csvexport.py b/binder/plugins/views/csvexport.py index 8d4cbf36..4d82d246 100644 --- a/binder/plugins/views/csvexport.py +++ b/binder/plugins/views/csvexport.py @@ -292,7 +292,7 @@ def get_datum(data, key, prefix=''): if fk_ids is None: # This case happens if we have a nullable foreign key that is null. Treat this as a many - # to one relation with no values. + # to one relation with no values. fk_ids = [] elif type(fk_ids) != list: fk_ids = [fk_ids]