diff --git a/pillar/shortcodes.py b/pillar/shortcodes.py index cdb93905..63104907 100644 --- a/pillar/shortcodes.py +++ b/pillar/shortcodes.py @@ -225,12 +225,25 @@ class Attachment: return self.render(file_doc, pargs, kwargs) - def sdk_file(self, slug: str, node_properties: dict) -> pillarsdk.File: + def sdk_file(self, slug: str, document: dict) -> pillarsdk.File: """Return the file document for the attachment with this slug.""" from pillar.web import system_util - attachments = node_properties.get('properties', {}).get('attachments', {}) + # TODO (fsiddi) Make explicit what 'document' is. + # In some cases we pass the entire node or project documents, in other cases + # we pass node.properties. This should be unified at the level of do_markdown. + # For now we do a quick hack and first look for 'properties' in the doc, + # then we look for 'attachments'. + + doc_properties = document.get('properties') + if doc_properties: + # We passed an entire document (all nodes must have 'properties') + attachments = doc_properties.get('attachments', {}) + else: + # The value of document could have been defined as 'node.properties' + attachments = document.get('attachments', {}) + attachment = attachments.get(slug) if not attachment: raise self.NoSuchSlug(slug) diff --git a/tests/test_shortcodes.py b/tests/test_shortcodes.py index 1201ed46..2e90cf28 100644 --- a/tests/test_shortcodes.py +++ b/tests/test_shortcodes.py @@ -187,37 +187,48 @@ class AttachmentTest(AbstractPillarTest): ], 'filename': 'cute_kitten.jpg', }) - node_doc = {'properties': { - 'attachments': { - 'img': {'oid': oid}, - } + + node_properties = {'attachments': { + 'img': {'oid': oid}, }} + node_doc = {'properties': node_properties} + + # Collect the two possible context that can be provided for attachemt + # rendering. See pillar.shortcodes.sdk_file for more info. + possible_contexts = [node_properties, node_doc] + # We have to get the file document again, because retrieving it via the # API (which is what the shortcode rendering is doing) will change its # link URL. db_file = self.get(f'/api/files/{oid}').get_json() link = db_file['variations'][0]['link'] - with self.app.test_request_context(): - self_linked = f'' \ - f'cute_kitten.jpg' - self.assertEqual( - self_linked, - render('{attachment img link}', context=node_doc).strip() - ) - self.assertEqual( - self_linked, - render('{attachment img link=self}', context=node_doc).strip() - ) - self.assertEqual( - f'cute_kitten.jpg', - render('{attachment img}', context=node_doc).strip() - ) + def do_render(context, link): + """Utility to run attachment rendering in different contexts.""" + with self.app.test_request_context(): + self_linked = f'' \ + f'cute_kitten.jpg' + self.assertEqual( + self_linked, + render('{attachment img link}', context=context).strip() + ) + self.assertEqual( + self_linked, + render('{attachment img link=self}', context=context).strip() + ) + self.assertEqual( + f'cute_kitten.jpg', + render('{attachment img}', context=context).strip() + ) - tag_link = 'https://i.imgur.com/FmbuPNe.jpg' - self.assertEqual( - f'' - f'cute_kitten.jpg', - render('{attachment img link=%r}' % tag_link, context=node_doc).strip() - ) + tag_link = 'https://i.imgur.com/FmbuPNe.jpg' + self.assertEqual( + f'' + f'cute_kitten.jpg', + render('{attachment img link=%r}' % tag_link, context=context).strip() + ) + + # Test both possible contexts for rendering attachments + for context in possible_contexts: + do_render(context, link)