Refactor requests out to a private function

[elliot-100]

Thinking about my missing one of the required changes to headers changes in #84 which meant get_events() didnn't authenticate...

I know it's difficult for this kind of library to run e.g. a proper fully-automated test suite on push/merge, as it depends on user authentication, but maybe a first step would be to have a standard script which can be run in local dev environment, includes logging and covers as much of the functionality as possible? (e.g. get names of all groups; use one of the group IDs to get one group; get a few events; use one of the group IDs to get one group)

I am kind of putting some of this together anyway for my own assurance, but I don't really use chat or write events, so it's not going to be complete... but I think I'll pull something together and comment what should be added.

(I know it's possible to unit test some of the behaviour, by e.g. mocking responses, but I think that would come later, as probably needs refactoring first.)

[Olen]

I agree.
Some kind of testing would be good.

In this specific case, I think another small refactor should be done, that moves all the requests out to a private function, to ensure they are all doing the same.

Right now, there are multiple variations, for example:

Group uses the auth_headers, sets a self.variable and returns it

        url = f"{self.api_url}groups/"
        async with self.clientsession.get(url, headers=self.auth_headers) as r:
            self.groups = await r.json()
            return self.groups

Chat uses some private headers and returns the await r.json()

        url = f"{self.chat_url}/chats/?max=10"
        headers = {"auth": self.auth}
        async with self.clientsession.get(url, headers=headers) as r:
            return await r.json()

Events does the same as Groups

        async with self.clientsession.get(url, headers=self.auth_headers) as r:
            self.events = await r.json()
            return self.events

UpdateEvents Uses its own headers, but returns self.events (and seems to be missing the auth-headers)

        headers = {"content-type": "application/json;charset=utf-8"}
        async with self.clientsession.post(url, json=data, headers=headers) as r:
            self.events_update = await r.json()
            return self.events

I feel that these should probably all use a common _post() and _get() and be consistent in what they return and how they behave...

something like

    def _get(url):
        async with self.clientsession.get(url, headers=self.auth_headers) as r:
        try:
            result = await r.json()
        except Whatever:
            Handle exceptions
            return None (or an empty dict?)
        return result

So the other functions can simply do

self.events = self._get(url):
return self.events

And ensure that the headers contain all the necessary parameters, charset in the content-type etc.

[elliot-100]

I guess the inconsistency in headers vs auth_headers proves my point - I simply got distracted after making the amends that I had confidence I could manually test, and then forgot to come back and fix the rest... 🤨

The refactor would prevent that in future, allow unit testing of a key component and resolve a lot of the inconsistencies (a bit of a chicken and egg problem as what should be returned isn't always documented).

But as a private function, I don't think it shouldn't have any effect on (or block writing of) an end-to-end manually reviewed test of output from the public functions - I missed out that key word.

[elliot-100]

The original issue ("Add some form of standard test for assurance when making changes") is resolved. But have kept + renamed, as the refactor proposal remains.