Skip to content

Conversation

scottlovegrove
Copy link
Contributor

@scottlovegrove scottlovegrove commented Aug 6, 2025

Fixes #307

Basically what was happening was the key for the reactions (ie, the emoji) was being passed from axios, to axios-case-converter then finally to camel-case, which was the stripping out emojis as they are not supported.

So, now we check to see whether the input is a single/composite emoji, if it is, we can assume this is a reaction and nothing else and not pass it through camel case.

The reason this wasn't caught by the tests, and why I have not added a test for this, is because all the tests are mocking out Axios completely, meaning the processing where this bug was happening doesn't actually happen. I'm not sure how to alleviate that. I have, at least, added some tests for the new customCamelCase function.

In order to test, create your scratch.ts file (see readme on how to run it), and put this in it:

import { TodoistApi } from './TodoistApi'

const token = 'YOUR_TOKEN'

const api = new TodoistApi(token)

async function doIt() {
    try {
        const comments = await api.getComments({ taskId: 'YOUR_TASK_ID' })
        console.log({ reactions: JSON.stringify(comments.results[0].reactions) })
    } catch (error: unknown) {
        if (error instanceof Error) {
            console.log(error)
        }
    }
}

doIt().catch(() => {
    // noop
})

Replace with your token, and a task id for a task that has a comment where that comment has a reaction.

@scottlovegrove scottlovegrove requested a review from a team August 6, 2025 09:25
@scottlovegrove scottlovegrove self-assigned this Aug 6, 2025
@scottlovegrove scottlovegrove requested review from Bloomca and removed request for a team August 6, 2025 09:25
Copy link
Contributor

@henningmu henningmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Perhaps we could consider a non-mocked axios for this test in restClient.test.ts. However, I don't think this is absolutely necessary 👍

@scottlovegrove
Copy link
Contributor Author

scottlovegrove commented Aug 6, 2025

Perhaps we could consider a non-mocked axios for this test in restClient.test.ts

Thinking about it, perhaps we could add msw in this instance 🤔 That would allow us to have the mock data without mocking Axios. But as you say, probably not necessary.

@scottlovegrove scottlovegrove merged commit e68d60b into main Aug 6, 2025
1 check passed
@scottlovegrove scottlovegrove deleted the scottl/fix-307 branch August 6, 2025 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getComment returns reactions object with an empty string key instead of the emoji
2 participants