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

Hide the draft from summary if cannot send to chat #6229

Closed
link2xt opened this issue Nov 19, 2024 · 8 comments
Closed

Hide the draft from summary if cannot send to chat #6229

link2xt opened this issue Nov 19, 2024 · 8 comments
Assignees

Comments

@link2xt
Copy link
Collaborator

link2xt commented Nov 19, 2024

This is a replacement issue for deltachat/deltachat-android#3428

There is a buggy state where you left the group or got removed, but the draft still exists. Draft is displayed in the summary, but is not accessible in the chat when you enter it.

To close the issue, we need:

  1. A test that if you set a draft, leave the chat or get removed, then get added back, the draft is still there.
  2. When you are not in the chat and can_send returns false, draft should not be displayed in the summary.
  3. Chatlist sorting should probably not take the draft into account. Most likely it will jump to the top because of member removed message anyway, so does not matter much, but in case UI sets the draft for non-writable chat, it should not be bumped to the top. Setting the draft to non-writable chat should probably not be allowed in the first place, otherwise UI may unset the draft because composer is not displayed.
@iequidoo
Copy link
Collaborator

Still, if the user set a draft, but then was removed from the group, i think it's necessary to make the draft accessible at least for copying, otherwise it's anyway a data loss from the user's point of view. It's not good that accessibility of the user's data is controlled remotely. There may be also a quote and attachment, but the text looks the most important thing.

@r10s
Copy link
Contributor

r10s commented Nov 21, 2024

sure, one can have different opinions on that, lots of things were discussed around that in the previous issues.

however, to move forward, we decided, that this is a minor issue with no real-life complains in 7 years, and we do not want to exchange that for additional complexity, maintenance and potentially worse issues.

on point: a note for closing this remainder: we must take care not to slow down the chatlist sql query; we did quite some effort to squeeze out some 1% speed, also this is not worth scarifying. if in doubt, just show an empty line instead of "draft: ..:" - i assume, however, that in most cases the "draft: ..." is not even shown as "group left" message was probably just not shown because of now fixed #6212

@adbenitez
Copy link
Collaborator

adbenitez commented Nov 21, 2024

It's not good that accessibility of the user's data is controlled remotely

lets get practical here, it is not like an user use the draft of a group to store important data that can be "controlled remotely", if you are drafting a message for a group, in the rare situation you get kicked out of the group permanently, then it is not important that you lost that draft

about this PR: if the draft is hidden and not accessible I think this virtually means it doesn't exist anymore for the user, and I think it is better to drop it for real, otherwise it is more unexpected if you get added back to the group again after some while that you see some old draft there and you don't know where it came from

@Septias
Copy link
Collaborator

Septias commented Nov 21, 2024

I like the idea of removing it completely aswell.

@Septias
Copy link
Collaborator

Septias commented Nov 21, 2024

will implement this new

@iequidoo
Copy link
Collaborator

iequidoo commented Nov 21, 2024

The most simple solution which doesn't require UI changes is to allow the user to finish the draft and send the last message. After that Chat::can_send() should start returning false. Anyway Delta Chat should deal with messages from non-members -- a message can be sent in offline mode when the user isn't already a member, a message can be sent from MUA which doesn't maintain the chat member list etc. This way no data, text/quote/attachment, is lost.

But adding any code path that discards the user's input means that users never can rely on that Delta Chat never loses data.

@Septias
Copy link
Collaborator

Septias commented Nov 22, 2024

I think losing a draft is not so bad, I personally don't use drafts very often, and if I have one it's probably by accident and it would be nice if someone removed it for me 😂. But I like the idea that we don't 'lose' data in any way so deleting a draft is maybe a bit harsh. On the other hand, it is not really clear what use a draft has that will not be received by anyone after you left/were removed from the group. It is probably also peculiar for a user who doesn't know what is happening if she is suddenly able to send to a group she thinks she is no part of. So I think our best bet is the current approach.

@Hocuri
Copy link
Collaborator

Hocuri commented Nov 23, 2024

This very small bug created a lot of discussions without any fix (which I wouldn't have expected when I brought it up), so, doing some issue gardening 🌱, let's move it to project resurrection.

@Hocuri Hocuri closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2024
@github-project-automation github-project-automation bot moved this to Closed PRs and Issues in Project Resurrection Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants