Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow an option for opting out of training data collection for each chat #3104

Merged
merged 1 commit into from May 13, 2023

Conversation

mtosity
Copy link
Contributor

@mtosity mtosity commented May 9, 2023

Issue: #2580

Add a button to support opt-out of training data.
A toggle would be nicer but currently can not get "opt-out status" from the list chat, only an PUT API to opt-out

Demo:

Recording.2023-05-08.234801.mp4

@mtosity mtosity marked this pull request as ready for review May 9, 2023 04:54
@olliestanley
Copy link
Collaborator

Just raised #3107 which should allow you to turn this into a toggle when merged

@notmd
Copy link
Collaborator

notmd commented May 9, 2023

Thank you. My idea for this is to introduce a tab on the right sidebar, one for config and one for common settings like title, allow_data_use, hidden, shared (in the future),... and the delete button. Then we will have spaces to explain allow_data_use mean, warn what happens with delete etc... Also avoid popups when clicking delete... Since on mobile, the chat list is already a popup, and triggering another popup is really bad practice IMO.

@mtosity
Copy link
Contributor Author

mtosity commented May 9, 2023

I could wrap the whole chat with a react context and the state will be { currentChatId: string}, then we could do chat common settings and chat config like you said

Also this will help to build the #2745

@notmd
Copy link
Collaborator

notmd commented May 9, 2023

I don't think we need a context for this. This can be implemented per chat seperatly.

@olliestanley
Copy link
Collaborator

What is the status of this? It seems like we can merge in current state and address concerns above in future PR(s), right? It is quite important to get this opt-out into prod asap.

@mtosity
Copy link
Contributor Author

mtosity commented May 13, 2023

Update from me: this PR still like the demo, a button to opt-out with a confirm modal. I can do a followup PR on this Sunday

@olliestanley olliestanley merged commit 972bb5d into LAION-AI:main May 13, 2023
4 checks passed
@olliestanley
Copy link
Collaborator

Update from me: this PR still like the demo, a button to opt-out with a confirm modal. I can do a followup PR on this Sunday

Thank you! It is now possible to get current opt out state from the backend so we could also convert to a toggle in this follow-up PR :)

@Maxuvious
Copy link

This 'opt out' button is very broken in its current state. If I inspect-element and delete a bunch of elements I can reach it and click it, otherwise it's unreachable, but even then there is no confirmation that the chat's data is actually opt-out of the data set. If this were the case, I would expect something like an indicator or even just that if you reselect the option a second time it might say something like 'opt this chat back into the data set', but alas, nothing of the sort. Should I open a new issue on this?
image

@Maxuvious
Copy link

This 'opt out' button is very broken in its current state. If I inspect-element and delete a bunch of elements I can reach it and click it, otherwise it's unreachable, but even then there is no confirmation that the chat's data is actually opt-out of the data set. If this were the case, I would expect something like an indicator or even just that if you reselect the option a second time it might say something like 'opt this chat back into the data set', but alas, nothing of the sort. Should I open a new issue on this? image

As well, from the screenshot I took, it says "Confirm opting of training data" which is unclear what this means -- opting out, or opting in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow an option for opting out of training data collection for each chat
4 participants