rbtnx / python-kanka

MIT License
3 stars 1 forks source link

Update entities #2

Open anthonyscorrea opened 3 years ago

anthonyscorrea commented 3 years ago

There doesn't seem to be a way to update entities. the upload() method doesn't work for this purpose. The readme says that updating is implemented for characters, but I'm not sure how to do it.

rbtnx commented 3 years ago

Hey, there isn't a dedicated update() method or anything, it just means you can locally alter the properties (name, age, etc) of an entity and change the entity on kanka.io successfully with the upload() method. I haven't been working on this repo for a while but I just tested it and it works. Feel free to ask me anything, if I didn't make myself clear :)

anthonyscorrea commented 3 years ago

Thanks for the response! Appreciate the help even though you haven't been working on it in a while.

When I try something like this:

character = self.campaign.character(character_id)
character.age = 100
response = character.upload()

The response variable shows a "404"/"Not Found" error. I believe there's an error with the endpoint/url. I think it hits the general api endpoint, but not the with the entity name endpoint in the url.

anthonyscorrea commented 3 years ago

Actually, if I run this command with the main branch, I get an UnboundLocalError: local variable 'r' referenced before assignment

From here:

def upload(self):
    """ Upload a new entity to the server. """
    if self.id is None:
        r = self.session.post(self.session.base_url, headers=self.session.headers, data=asdict(self))
        if r.status_code == 201:
            return r.json()
    return r

Appears as though it's design to work on a new entity, not an existing entity. I forgot that I modified this with another outcome on the if self.id gate. Here's what I changed it to:

if self.id is None:
    r = self.session.post(self.session.base_url, headers=self.session.headers, data=asdict(self))
    if r.status_code == 201:
        return r.json()
else:
    r = self.session.put(self.session.base_url, headers=self.session.headers, data=asdict(self))
    if r.status_code == 201:
        return r.json()
return r

Not sure if this is against intent. Any feedback is helpful. Thanks again.

rbtnx commented 3 years ago

You're right, upload() actually works only for entities without an id.. I think it would be best to implement an update() method for existing entities so there is no confusion. I'll work on it next, I'll also try to update the repo (had some issues with missing field errors, maybe there where some changes to the api).

Is your change to the upload() method working for you? Are there any other issues? Feedback is greatly appreciated :)

anthonyscorrea commented 3 years ago

My change doesn't work because the url for updating is different than the url for updating is different than the one for creating a new entity. You need to include the /{child_entity}/{child_id} endpoint for a successful put request.

I can work on this too. If you create an issue for the missing fields, I can help with that too.

I was having an issue where having "None" for one of the fields made the type checking fail. I think it was since there was no image path for a particular entity, it failed that that field was NoneType and not a str. I got all confused on how to make dacite happy. I'll have to work to duplicate that issue and open another issue.

rbtnx commented 3 years ago

I fixed the issue when image_full is None, see the bugfixes branch, I'll open an issue for the error I discovered. I can work on the update() method tomorrow

anthonyscorrea commented 3 years ago

Oh, I see how you fixed "image_full", I think maybe I got confused with "image" and missed the default assignment to None. Cool. Thanks.