linuxserver / docker-dokuwiki

GNU General Public License v3.0
110 stars 24 forks source link

Persist smileys/local and interwiki image folders outside of container #41

Closed MachX428 closed 1 year ago

MachX428 commented 2 years ago

Fixes custom smiley and interwiki icon support

linuxserver.io



Description:

Following the official DokuWiki documentation for working with custom smileys (https://www.dokuwiki.org/smileys) and interwiki icons (https://www.dokuwiki.org/interwiki) does not result in the custom icons being displayed. This is because the folders the user is supposed to put the icons into are not properly persisted outside of the docker container. This PR modifies the 50-config file to make these locations persistent.

Benefits of this PR and context:

This fixes an outstanding bug and closes #40.

How Has This Been Tested?

The change has been tested by:

===== Smiley =====

:SMILEYTEST:

  * Verify that the text ":SMILEYTEST:" and a generic icon appears
    * **If it was working correctly, the smiley image you saved would appear instead**
  * Create an interwiki folder as described in the official documentation: https://www.dokuwiki.org/interwiki
    * `config/dokuwiki/lib/images/interwiki`
  * Use your favorite paint program to create a 16x16 pixel red square and save it as ubuntu.png
    * Move it to the interwiki folder you created
  * Create a `config/dokuwiki/conf/interwiki.local.conf` file 

Testing Interwiki's

ubuntu https://wiki.ubuntu.com/{NAME}

  * Edit your DokuWiki test page and add the text "[[ubuntu>teams|Ubuntu Teams]]"  

====== Test ======

===== Smiley =====

:SMILEYTEST:

===== Interwiki =====

[[ubuntu>teams|Ubuntu Teams]]

  * Verify that a generic circle icon appears next to the interwiki link you created
    * **If it was working correctly, the red square icon you created would appear instead**
  * Build new (local) image from this PR.
  * Shut down and remove the existing container (but don't delete your persistent folders)
  * Clear the image cache: 

rm -rf config/dokuwiki/data/cache//.{gif,png,css}


  * Create and start up new container using the local image you just created
  * Go back to your DokuWiki test page
    * You may have to force refresh  
  * **The correct smiley image and red interwiki icon should now be visible**

## Source / References:
<!--- Please include any forum posts/github links relevant to the PR -->
See issue #40.
LinuxServer-CI commented 2 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2020-07-29-pkg-5739847a-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2020-07-29-pkg-5739847a-pr-41/shellcheck-result.xml

github-actions[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

MachX428 commented 2 years ago

Is there anything I need to do in order to get this approved? It would be very useful to get this fixed in the repo.

github-actions[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

MachX428 commented 2 years ago

I'm just commenting to indicate I still have interest in this. Is there anything that I can do to help get this approved?

github-actions[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

thelamer commented 2 years ago

This doesn't work for me without:

  [[ ! -d /config/dokuwiki/lib/images/smileys ]]
    mkdir -p /config/dokuwiki/lib/images/smileys

Did you test this ?

LinuxServer-CI commented 2 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2020-07-29-pkg-c3609974-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2020-07-29-pkg-c3609974-pr-41/shellcheck-result.xml

MachX428 commented 2 years ago

Thank you. I did indeed check it, but in retrospect my test process was flawed. I started with the non-modified repo, went through the test steps to create the smileys/interwiki's and verified they DIDN'T work. I then rebuilt the repo with the changes, and verified that the previous non-functioning pages now worked. However, since my steps had added the /config/dokuwiki/lib/images folder, it masked the fact I needed to check for that in the 50-config file as well.

I have added the following to 50-config:

[[ ! -d /config/dokuwiki/lib/images/ ]] mkdir -p /config/dokuwiki/lib/images/

and merged/rebased with the latest. I then started completely from scratch, with no existing folders, rebuilt the container with the changes, ran it, and verified everything works as expected. I then updated my PR branch with the changes. Thanks again for pointing this out.

github-actions[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

LinuxServer-CI commented 2 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2020-07-29-pkg-cd7de6ad-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2020-07-29-pkg-cd7de6ad-pr-41/shellcheck-result.xml

nemchik commented 1 year ago

@MachX428 this looks fine to me, but it needs to be rebased at this point. Can you rebase and then ping me back?

github-actions[bot] commented 1 year ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/shellcheck-result.xml

MachX428 commented 1 year ago

@nemchik - I rebased as per requested, and validated all works as expected.

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/shellcheck-result.xml

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/shellcheck-result.xml

nemchik commented 1 year ago

lspipepr/dokuwiki:2022-07-31a-pkg-5b04a879-pr-41 would be the tag to test.

MachX428 commented 1 year ago

@nemchik - I tested that tag and all works well.

That being said, I'm thinking one more slight change might make this just a little more robust and have less possibility for confusion.

When I originally did the change, I just created the base lib/images directory. If you then follow the official smiley instructions here https://www.dokuwiki.org/smileys and create the /smileys/local directory under /images, and put your smileys there, it works. I figured anybody creating the images/smileys directory would obviously be reading the instructions and also know to create the /local directory under it. But now with your changes, it creates actually the /images/smileys directory for you. And at this point it almost makes it seem like you can just put the custom smileys there since that directory exists. Of course if you do that, it won't work because the symlink is to /images/smileys/local as per the official instructions. So my suggestion is we just bite the bullet and create the whole /images/smileys/local path at this point to just avoid any confusion. It is more intuitive and probably what I should have just done in the first place. I am happy to make the change, test it, and push it up for you if you agree.

MachX428 commented 1 year ago

I went ahead and made the change I described, and retested. All looks well on my end if you want to add that in.

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/index.html https://ci-tests.linuxserver.io/lspipepr/dokuwiki/2022-07-31a-pkg-5b04a879-pr-41/shellcheck-result.xml