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

fix(block): remove chat and messages when blocking a contact #16889

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrainville
Copy link
Member

@jrainville jrainville commented Dec 5, 2024

What does the PR do

Fixes #16640

This makes it so that when you block a contact, it now also removes the chat and the messages as expected by the requirements and as Mobile does.

To do so, I use the same API as mobile instead of the forked desktop one. I removed the desktop one as it is no longer needed (see status-go PR)

I also fixed an issue when unblocking where it would send a double toast messages with one saying you "removed the contact", but it was already removed.

Affected areas

Chat and contacts

Architecture compliance

Screenshot of functionality (including design for comparison)

block-user.webm

Impact on end user

Removes the chat and message when blocking

How to test

  • Block a contact that had sent messages
  • Unblock them
  • Add them again to see the 1-1 (no more messages from them [except the new CR])

Risk

Tick one:

  • Low risk: 2 devs MUST perform testing as specified above and attach their results as comments to this PR before merging.
  • High risk: QA team MUST perform additional testing in the specified affected areas before merging.

Not much. Worst case some parts of the blocked user still appear until a restart.

@jrainville jrainville requested a review from a team as a code owner December 5, 2024 15:45
@jrainville jrainville requested review from noeliaSD, a team, iurimatias, caybro and igor-sirotin and removed request for a team December 5, 2024 15:45
@jrainville jrainville removed the request for review from noeliaSD December 5, 2024 15:46
@status-im-auto
Copy link
Member

status-im-auto commented Dec 5, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 487ffca #1 2024-12-05 15:54:04 ~7 min tests/nim 📄log
✔️ 487ffca #1 2024-12-05 15:58:07 ~11 min macos/aarch64 🍎dmg
✔️ 487ffca #1 2024-12-05 15:58:26 ~12 min tests/ui 📄log
✔️ 487ffca #1 2024-12-05 16:02:51 ~16 min linux-nix/x86_64 📦tgz
✔️ 487ffca #1 2024-12-05 16:06:17 ~19 min macos/x86_64 🍎dmg
✔️ 487ffca #1 2024-12-05 16:06:46 ~20 min linux/x86_64 📦tgz
✔️ 487ffca #1 2024-12-05 16:08:41 ~22 min windows/x86_64 💿exe
✔️ 9400674 #2 2024-12-12 18:53:55 ~5 min macos/aarch64 🍎dmg
✔️ 9400674 #2 2024-12-12 18:56:19 ~8 min tests/nim 📄log
✔️ 9400674 #2 2024-12-12 19:00:35 ~12 min tests/ui 📄log
✔️ 9400674 #2 2024-12-12 19:03:01 ~15 min linux-nix/x86_64 📦tgz
✔️ 9400674 #2 2024-12-12 19:04:59 ~16 min macos/x86_64 🍎dmg
✔️ 9400674 #2 2024-12-12 19:06:27 ~18 min linux/x86_64 📦tgz
✔️ 9400674 #2 2024-12-12 19:10:42 ~22 min windows/x86_64 💿exe

src/app_service/service/contacts/service.nim Show resolved Hide resolved
singletonInstance.globalEvents.showContactRemoved("Contact removed", fmt "You removed {contact.displayName} as a contact", contact.id)
signal = SIGNAL_CONTACT_REMOVED
if contact.removed and not self.contacts[publicKey].dto.removed:
singletonInstance.globalEvents.showContactRemoved("Contact removed", fmt "You removed {contact.displayName} as a contact", contact.id)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to do the notification here at all; we do those (in the correct format) in Popups.qml already. There are different permutations, depending on what other actions the user takes via the popups (like marking as untrosted, blocking, etc). All of these are processed at once, so that you don't get a single notification for each action.

Copy link
Member

Choose a reason for hiding this comment

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

Also (not a big deal atm) this is not translatable at all

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true. I guess we have a couple of stray notifications in Nim. I don't know if we have already an issue to refactor those to move them to QML

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there was some initial effort by Noelia (see ToastsManager.qml) but never finished actually; I guess here's one of those (many) still done on the NIM side

Fixes #16640

This makes it so that when you block a contact, it now also removes the chat and the messages as expected by the requirements and as Mobile does.

To do so, I use the same API as mobile instead of the forked desktop one. I removed the desktop one as it is no longer needed (see status-go PR)

I also fixed an issue when unblocking where it would send a double toast messages with one saying you "removed the contact", but it was already removed.
@jrainville jrainville force-pushed the fix/block-user-deletes-messages branch from 487ffca to 9400674 Compare December 12, 2024 18:47
@jrainville
Copy link
Member Author

@caybro @igor-sirotin @iurimatias friendly reminder to review

Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

LGTM

I'll have a look at the notification within the other issue assigned to me

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.

Blocking a user does not purge the database of received messages
3 participants