andrewthetechie / geekbot-api-py

A Geekbot (https://geekbot.com/) API client in python supporting async
MIT License
1 stars 2 forks source link

Report class has incorrect field name #19

Closed rmathew8-kust closed 1 year ago

rmathew8-kust commented 1 year ago

It appears that the Report class has an incorrect field name.. and there is no test coverage for this. The below change seems to fix the issue.

class Report(BaseModel):
    id: int
    slack_ts: Optional[str]
    standup_id: str
    timestamp: int
    channel: str
    is_anonymous: Optional[bool]
    member: User
    questions: List[Answer]
    # answers: List[Answer]
andrewthetechie commented 1 year ago

Hi Roy,

Thanks for the report. I'd happily approve a PR that makes this change and adds testing for it!

rmathew8-kust commented 1 year ago

Hi - I'm trying to model my test for reports on the teams test, and am a bit unclear about the below.. could you clarify. thanks

def test_get_teams_one_team(test_client, fake_team):
    test_client["requests_mock"].get(
        # shouldn't this be json=[fake_team.dict()]
        url=test_client["client"].teams._get_request_path(), json=fake_team.dict()
    )
   ...
andrewthetechie commented 1 year ago

Hi Roy,

That test is testing that when the API returns a single team, instead of a list, that the code operates properly. I don't think the reports endpoint does that - in all of my testing it always returns a list.

I wish geekbot released a better documented API and had a changelog - I don't know when they changed from "questions" to "answers" in their returned object.

I don't do a lot of work with the geekbot API these days, so this has potentially been broken for a while

andrewthetechie commented 1 year ago

https://github.com/andrewthetechie/geekbot-api-py/pull/21

andrewthetechie commented 1 year ago

https://github.com/andrewthetechie/geekbot-api-py/releases/tag/v0.2.1

Thanks for the fix!