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 diff --git a/binder/plugins/views/csvexport.py b/binder/plugins/views/csvexport.py index 6e66d2ff..4d82d246 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 @@ -284,7 +289,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..8a069a03 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) @@ -54,12 +56,12 @@ def test_csv_download(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_excel_download(self): @@ -74,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)]) @@ -95,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') @@ -115,12 +117,55 @@ 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.pictures[2].id, str(self.pictures[2].animal_id), (self.pictures[2].id ** 2)]) \ No newline at end of file + 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): + """ + 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) + + 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/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..9c7ffe36 100644 --- a/tests/testapp/views/picture.py +++ b/tests/testapp/views/picture.py @@ -3,16 +3,24 @@ 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'), + ('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'])