xwikisas / xwiki-pro-macros

XWiki rendering macros useful when migrating content from Confluence
GNU Lesser General Public License v2.1
3 stars 12 forks source link

Confluence gallery macro doesn't display images by providing page reference that has them #172

Open petrenkonikita112263 opened 1 year ago

petrenkonikita112263 commented 1 year ago

Prerequisites:

Step to reproduce:

  1. Upload custom images (or even several of them) into any Test pages
  2. Navigate to Sandbox page and call the Confluence Gallery macro from CKEditor toolbar menu. Do not provide any inputs; leave them empty (this step is just for testing displaying the attached default xwiki logo on Sandbox page)
  3. Call this the 2nd time, but this time provide the title "Test Gallery" and page reference "Sandbox.TestPage1" or any other name where you uploaded custom images

Actual result The first macro call will display XWiki logo as was mentioned and have this code {{confluence_gallery/}}, but second call won't display images with code {{confluence_gallery title="Test Gallery" page="Sandbox.TestPage1"/}}.

image

Note that if you copy/paste simply line {{confluence_gallery/}} into test page, you'll see desired images in a box.

Expected result The macro displays images that are located on another page.

raphj commented 11 months ago

@oanalavinia

This is probably because of this:

    #set ($query = $services.query.xwql('where doc.title = :name').bindValue('name', $page))

I expect page to store an actual document name. Note also these changes that will probably fix whatever you were fighting against while writing this code:

I think we will need to remove the XWQL query when CONFLUENCE-153 is fixed. WDYT?

oanalavinia commented 11 months ago

Hi @raphj ! The last change of that line is part of a larger commit where I added the license check for all pro macros, which meant adding indentation. The query was actually added as part of https://github.com/xwikisas/xwiki-pro-macros/commit/8ecfafb7c18adf42aaffb560165218de07fb18be by @Vertganti , maybe he can give an input on the matter :)

Vertganti commented 11 months ago

Note that the confluence_gallery macro is only a wrapper for the standard XWiki gallery macro. The confluence_gallery macro is intended for pages imported from Confluence. It is not indended to be added manually by a user (which the original post seems to be about). It has been way too long since I wrote that code to remember exactly why i wrote it that way. But I took another look at the relevant code:

#foreach ($page in $pages)
  ## Resolve page and put attachments in temporary list
  #set ($tempAttachments = [])
  #set ($query = $services.query.xwql('where doc.title = :name').bindValue('name', $page))
  #set ($pageStrings = $query.execute())
  #if ($pageStrings.size() > 0)
    #set ($pageDocument = $xwiki.getDocument($pageStrings[0]))
    #set ($discard = $tempAttachments.addAll($pageDocument.getAttachmentList()))

We iterate over all page names listed in the page parameter. For each of those we get the first document whose title matches the specified page name. IIRC Confluence identifies pages based on the titles, so there should be no duplicates in an imported space. We then take all attachments of the document and continue with the rest of processing, which is not relevant here.

The problem I see is that @petrenkonikita112263 specified the page by its full path Sandbox.TestPage1 while the macro uses only the title for identifying the page as it is only a confluence wrapper. I am sure the macro could be extended to support paths (using the changes mentioned by @raphj ). It might even be necessary if you import multiple Confluence spaces into a single XWiki (sub)wiki to avoid issues if pages have the same titles.

Actually, after writing all this I checked up on our internal documentation on the macro and noticed that I changed the relevant lines there. Apparently I already stumbled upon this exact issue since the updated code seems to adress it. I cannot remember why I did not update the macro here, might have forgotten it. The updated code is:

#foreach($page in $pages)
    ## Resolve page and put attachments in temporary list
    #set ($tempAttachments = [])
    #set($query = $services.query.xwql("where doc.fullName like :name").bindValue('name', '%'+${page}+'%'))
    #set($pageReference = $query.execute())
    #set($pageDocument = $xwiki.getDocument($pageReference))
    #set($discard = $tempAttachments.addAll($pageDocument.getAttachmentList()))

The size check used as a safety guard is probably missing because of the aforementioned assumption that there are no duplicate titles in Confluence. I can PR the changes if required.

To reiterate: The confluence_gallery macro is not intended for adding a gallery manually through XWiki. If you want to add a gallery to an XWiki page manually you should use the standard gallery macro, which supports specifying images by path as far as I can see.

snazare commented 3 months ago

tested on XWiki 15.10.10, Pro Macros 1.19.9, gallery bridge does not show anything post import. Not sure it's related but we should look into it.