Allow to monitor all changes in repositories of an user

[mharrend]

Dear guys, thank you very much for your helpful Github integration. However, I would propose a small function extension:

Current status

If I want to monitor the repositories of an user, I have to define each user repository as a single line in the config.py file. If one has lots of repositories this is quite time-consuming.

Updated version

I can still set the behaviour for each user repository individually, but if the user repository has no individual configuration. A global configuration for all repositories of an user (if defined) is used. Furthermore, I replaced the if statements with elif statements which is IMHO a little bit more the python way.

[ptersilie]

:+1: Thanks a lot for the PR. Good idea to filter for owners as well. We'll have to do a few changes though, as this PR currently breaks some things. The reason I didn't use elif is so that we can always return the default hook, even if the payload has an organisation but no filter was defined by it. Currently (in this PR) a payload for an organisation repository that is not defined in config.py is simply ignored.

I suggest we change the code to something like this:

def get_hook_info(data):
    if 'repository' in data:
        repo = data['repository']['full_name']
        if repo in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[repo]
    if 'organization' in data:
        org = data['organization']['login']
        if org in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[org]
    if 'repository' in data:
        if data['repository']['owner'].has_key("login"):
            owner = data['repository']['owner']['login']
            if owner in config.MATTERMOST_WEBHOOK_URLS:
                return config.MATTERMOST_WEBHOOK_URLS[owner]
    return config.MATTERMOST_WEBHOOK_URLS['default']

I currently don't have webhooks setup on a non-organisation account. Would you mind updating the PR and testing if this works for you?

[mharrend]

I checked your proposal and looks fine. However, during my checks i found out that in some cases the Github api does not send the login name of the owner of the repository and instead sends the name of the owner. So the complete code which works for me looks like this:

def get_hook_info(data):
    if 'repository' in data:
        repo = data['repository']['full_name']
        if repo in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[repo]
    if 'organization' in data:
        org = data['organization']['login']
        if org in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[org]
    if 'repository' in data:
        if 'login' in data['repository']['owner']:
            owner = data['repository']['owner']['login']
            if owner in config.MATTERMOST_WEBHOOK_URLS:
                return config.MATTERMOST_WEBHOOK_URLS[owner]
        if 'name' in data['repository']['owner']:
            owner = data['repository']['owner']['name']
            if owner in config.MATTERMOST_WEBHOOK_URLS:
                return config.MATTERMOST_WEBHOOK_URLS[owner]
    return config.MATTERMOST_WEBHOOK_URLS['default']

[ptersilie]

Good spot. The Github API is a bit arbitrary sometimes.

I think we can shorten this a bit:

def get_hook_info(data):
    if 'repository' in data:
        repo = data['repository']['full_name']
        if repo in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[repo]
    if 'organization' in data:
        org = data['organization']['login']
        if org in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[org]
    if 'repository' in data:
        owner = None
        if 'login' in data['repository']['owner']:
            owner = data['repository']['owner']['login']
        elif 'name' in data['repository']['owner']:
            owner = data['repository']['owner']['name']
        if owner in config.MATTERMOST_WEBHOOK_URLS:
            return config.MATTERMOST_WEBHOOK_URLS[owner]
    return config.MATTERMOST_WEBHOOK_URLS['default']

I just saw that organisations also have owner set (sometimes name sometimes login). So we actually might be able to drop the organization check completely. But I'll probably do that in another PR.

Do we need to update the Readme for this or does the config for owners work exactly like organisations?

[mharrend]

+1 Looks fine for me. Yes, there is no need to change the Readme.

BTW: I will open a new PR in an instant since I found a way to fix the handling of avatars considering your comments on the issue.

[ptersilie]

Merged.