Docker-manager plugin not linking to github properly


(David Taylor) #1

I have a standard docker-based installation, and use the docker-manager plugin to keep things up to date. This works fine for most plugins, but for some reason some of the plugins don’t work properly. The link to the commit is wrong, but actually updating works fine.

So, for the discourse-solved plugin, the generated HTML looks like

<td>
  discourse-solved
  (<a href="https://github.com/discourse/discourse-solved/compare/9936707...master">9936707</a>)
</td>

and that works fine.

However, for the discourse-push-notifications plugin, it looks like this

<td>
  discourse-push-notifications
  (<a href="/compare/003519a...master">003519a</a>)
</td>

The problem isn’t isolated to that plugin, it also happens for discourse-migrate-password, and a plugin of my own.

All the plugins were initially installed by including a git clone line in the app.yml container definition. As far as I can tell they are all in the same state as each other so no idea what’s causing the difference.

If I enter the docker container, and go to the directory of the plugins, running git config --get remote.origin.url returns

https://github.com/discourse/discourse-push-notifications

There are no errors in the javascript or server logs. Is there anywhere else I can look to try and figure out what’s going on?


(cpradio) #2

I can’t recreate this. I added discourse-push-notifications to my instance and it doesn’t exhibit this behavior.

If you open dev tools, do you see any errors?


(David Taylor) #3

Got nothing in the javascript console, and don’t really have any idea how to investigate the problem further. The issue persists for me even after a full rebuild of the container.


(cpradio) #4

What other plugins do you have installed (can you copy and paste that section of your app.yml)? Have you made any customizations to the /var/discourse/templates area?

… trying to think of other possibilities…

Have you tried using incognito mode with all extensions disabled?

What version of Discourse are you running? and which branch are you following?


(David Taylor) #5

Think I’ve tracked it down. It breaks for repositories that have been cloned without the URL ending in .git

(I cloned by doing git clone https://github.com/discourse/discourse-push-notifications instead of https://github.com/discourse/discourse-push-notifications.git)

I believe the line causing this is line 9 here:

If the URL doesn’t contain .git then url.indexOf returns -1, and so url.substr(0,-1) returns an empty string.

Something like this might work (but there’s probably a tidier way):

var extension = url.indexOf('.git')
if (extension == -1){
    var _url = url  	
}else{
    var _url = url.substr(0, extension);	
}

Unfortunately I can’t manage to build docker-manager/manager-client in my dev environment so I can’t test this or do a pull request :frowning:


(cpradio) #6

Cool, that sounds like a simple repro step. I’ll look into submitting that PR once I recreate it :slight_smile: Might not happen today though.


(cpradio) #7

PR created


(David Taylor) #8

Any chance of getting this merged in @eviltrout @cpradio?


(Robin Ward) #9

It looks like the assets weren’t compiled yet. I posted another comment on the commit.


(cpradio) #10

No, they are. I added them to the PR a while ago. I’ve been using my branch in my sandbox for over a month without any issues.


(Jeff Atwood) #11