Logout feature added - Githubissues

xtreamsrl / clockify-nlp-bot

An NLP driven bot for interacting with Clockify timesheet via different channels

MIT License

10 stars 3 forks source link

Logout feature added #122

Open swissbyte opened 2 years ago

swissbyte commented 2 years ago

This is the merged #106 feature which a branch already existed for. But the existing branch lacked the functionality of removing the tokens from the key vault. This branch removes the token from:

local in memory storage (also from the persistent one)
local cache if the key vault is used
key vault

Would be nice if you accept the request :)

khaelys commented 2 years ago

Thank you for your contribution! I'll check this when I return from vacation ☺️

LorenDB commented 2 years ago

Is this coming anytime soon?

swissbyte commented 2 years ago

@LorenDB you can check my repository... there you can find a version with the logout feature already integrated

LorenDB commented 2 years ago

@LorenDB you can check my repository... there you can find a version with the logout feature already integrated

The problem is that the xstream-hosted instance doesn't have it merged. 😕

swissbyte commented 2 years ago

I was not aware that there is a hosted version... and why dont you host it on your own azure instance?

LorenDB commented 2 years ago

Because (a) I was not entirely aware that self-hosting was an option and (b) I was just testing out the system to see how I liked it.

swissbyte commented 2 years ago

Self hosting is very easy. Check out my repo. I updated the readme.md with a lot of additional information. Also regarding self hosting

khaelys commented 2 years ago

Is this coming anytime soon?

@swissbyte I'll try to look at this PR this evening, but I can't give you any guarantee, we are busy in this period :)

khaelys commented 2 years ago

Hi @swissbyte I started reviewing your PR, but it looks like you added other features, like set working hours, an improvement on reminders and an improvement on the documentation.

In this PR I would like to review only the logout feature (I guess the last 3 commits). Can you cherry-pick them and resubmit the PR? The other features look stunning, but I would like to review and add them one at a time :)

swissbyte commented 2 years ago

Hi @khaelys thanks for answering. Sure, i thought it would only include the commits up to the date when k did the pull request 😅

Damn... so i have to get into cherry picking... i will have a look at it tomorrow

If you have a small step by step, i would be glad to get this...

K know basically what cherry picking is but not in conjunction with a PR

khaelys commented 2 years ago

@swissbyte I am not comfortable with it either, but I thought about two options:

cherry-pick the last 3 commits in a new local branch, point to the same remote branch, and force push.
cherry-pick the last 3 commits in a new local branch, push to a new or the same remote branch, but you open a new pull request

One more thing, I noted that there aren't tests, but It may be the case to add some, like a couple of tests for the InMemoryTokenRepository and TokenRepository, you'll find the suites under Bot.Tests/Data.

It will be nice to also start unit testing dialogs as we go. You can look at Bot.Tests/Data/Dialogs/ClockifySetupDialogTest for some ideas on how to do it.

If you need help let me know!

swissbyte commented 2 years ago

Thanks for the answer. Yes, k defeniteley have to dive into tests. I know the theory behind and did some during studies but i have not weitten that much cases before...

LorenDB commented 2 years ago

Ping!

I'm still hoping that this PR will get merged...