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.

Sign up for GitHub

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

Jump to bottom

SES-1156 - Ban and delete functionality fix #1428

Merged
merged 21 commits into from
Apr 2, 2024
Merged

SES-1156 - Ban and delete functionality fix #1428

merged 21 commits into from
Apr 2, 2024

Conversation

AL-Session
Copy link
Collaborator

Contributor checklist

  • I have tested my contribution on these devices:
  • Device Samsung S9+, Android 9, API 28
  • Virtual device Pixel 3a, Android 9, API 28
  • Virtual device Pixel 3a, Android 14, API 34
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Ban and delete functionality when used on users of a community did not always remove the messages from the local storage databases on Android (although the messages WOULD always be deleted server-side). This resulted in 'zombie' messages existing on a local device which could not be deleted (the messages essentially had no recipient, and the delete functionality would just fail silently).

To address this issue the ban and delete functionality has been modified so that the device performing the "Ban and delete" operation takes a pass over the local storage identifying all messages by the banned user and removing those messages from local storage, thus keeping the banning device in sync with the correct state of the community's messages (where the banned users messages will have been removed without issue).

To test this PR, ban and delete a user from a community, whether from a message they've sent or from a message they've replied to, and observe:

  • The banned user's messages are deleted from the server-side data (an easy way to observe this is via the desktop client),
  • The banned user's messages are deleted from the Android device performing the 'Ban & delete' operation (this is essentially "the fix"), and
  • The banned user's message deletion is reflected in updates to any other community members using the Android client (as before).

0x330a
0x330a reviewed Mar 13, 2024
View reviewed changes
app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt Outdated
@@ -144,8 +151,13 @@ class ConversationViewModel(
}

fun deleteForEveryone(message: MessageRecord) = viewModelScope.launch {
val recipient = recipient ?: return@launch
val recipient = recipient
if (recipient == null) {
Copy link

Choose a reason for hiding this comment

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

We might be able to return the Log.w call from here to save this check, something like
val recipient = recipient ?: return@launch Log.w(...

@mpretty-cyro mpretty-cyro changed the title SA1416 - Ban and delete functionality fix SES-1156 - Ban and delete functionality fix Mar 21, 2024
@AL-Session AL-Session merged commit a8a257a into oxen-io:dev Apr 2, 2024
1 check was pending
AL-Session added a commit that referenced this pull request May 1, 2024
@bemusementpark @alansley
@0x330a @AL-Session
Merge dev (#1468)
d16faf9 
* Remove unused sizeResId

* Fix caching

* Prefix message with name in HomeActivity

* Hide sender prefix for note to self

* Hide sender prefix for control messages

* Remove problematic getLastMessage()

* Refactor snippet formatting

* Remove unused RecoveryPhraseRestoreActivity

* Fix unresolved theme attributes exception

* Fix dialog button style

* Investigation in progress

* Working fix push before cleanup

* Fixes signalapp#1346

* Removed unused logging imports

* Put back some whitespace

* Minor cleanup

* Fix NPE on null display name

* fix: disappearing viewmodel tests (#1432)

* SES-1354 - Video call self viewer not mirrored (#1397)

* Fixes signalapp#874

* Removed accidentally left in line

* Fixed issue - push before cleanup

* Cleaned up

* Removed cruft

---------

Co-authored-by: = <=>
Co-authored-by: AL-Session <[email protected]>

* SES-1145 - New messages are hidden under keyboard - MK3 (#1415)

* WIP

* Working - push before cleanup

* Fixes signalapp#1316

* Cleanup

* PR review adjustments

* Fixed scrolling when receiving an image based message while keyboard is up

* Prevent auto-scroll to last seen item pos in conversation view if <= 3

* Put back <=3 check to scroll

---------

Co-authored-by: = <=>
Co-authored-by: AL-Session <[email protected]>

* Fix missing parenthesis

* SES-789 - Scroll to bottom of long new message(s) (#1426)

* WIP

* Working - push before cleanup

* Fixes signalapp#1316

* Cleanup

* PR review adjustments

* Fixed scrolling when receiving an image based message while keyboard is up

* Prevent auto-scroll to last seen item pos in conversation view if <= 3

* Put back <=3 check to scroll

* Forced scrolling to bottom of long messages (both sent and received) when already at the bottom of the RecyclerView

* Fixes signalapp#1364

---------

Co-authored-by: = <=>
Co-authored-by: AL-Session <[email protected]>

* SES-1352 - User and group names allowing multi-line strings (#1395)

* Fix WIP

* Resolved issue - pushing before cleanup & PR tomorrow morning

* Enforced single line for new closed group names

* Fixes signalapp#1394

* Final cleanup prior to PR

* Added code to restore a previous contact nickname if an empty one is given

* Added initial limits to nicknames and group names, both creation and display

* Minor adjustments

* Adjusted max nickname and group name to 35 chars as per Kee's instructions

* Fixed closed group edit text able to get too wide and cut off buttons

---------

Co-authored-by: = <=>
Co-authored-by: AL-Session <[email protected]>
Co-authored-by: Al Lansley <[email protected]>

* SES-212 - Always show delivery status of last sent message - FINAL! (#1418)

* Fixes signalapp#1408

* Addressed PR feedback

* Cleanup

* PR adjustments

* Further PR adjustments

* Updated libsession-util

* Added fix for crash when no messages

* Ignoring dirty submodules so they don't show up in git

* Re-fixed display of delivery status on last sent message (got broken by disappearing messages)

* Removed ignore dirty modules line in .gitmodules as it all seems to be playing nice now

---------

Co-authored-by: AL-Session <[email protected]>
Co-authored-by: Al Lansley <[email protected]>

* fix: use a set for the from/to serialized lists (#1370)

* Fixes signalapp#1347 (#1396)

Co-authored-by: AL-Session <[email protected]>
Co-authored-by: Al Lansley <[email protected]>

* SES-1156 - Ban and delete functionality fix (#1428)

* WIP

* Investigation in progress

* End of day push

* WIP

* Fixes signalapp#1416

* Cleanup

* Added code to remove zombie messages caught in limbo during a ban & delete - still chock full o' debug while finding root cause

* Root cause debug WIP

* Push prior to cleanup

* Cleaned up for PR

* fix: mms delete, remove unnecessary values from sms

* Addressed PR feedback

* fix: fix unit tests

* Added '.run' folder with test setup

* Update README.md

Test commit for CI

* Re-added accidentally removed closing brace

---------

Co-authored-by: alansley <[email protected]>
Co-authored-by: Al Lansley <[email protected]>
Co-authored-by: 0x330a <[email protected]>

* SES-1356 - List of recently used reaction emojis is not accurate (#1400)

* WIP

* Further WIP

* Push prior to cleanup

* Fixes signalapp#1015

* Added limiting to the count of recently used emoji that we store

* Put back adjusted reaction pill layout to standard

* Adjusted recently used reaction emojis already in list to go to start of list

---------

Co-authored-by: = <=>

* SES-697 - Add loading state when exporting logs (#1402)

* WIP

* Fixes signalapp#1401

* Cleanup from PR view

* Final cleanup

* Removed commented line of code & re-ordered comment

* Addressed PR feedback

* Re-allowed loading of avatars to throw exceptions rather than return null on failure

---------

Co-authored-by: = <=>

* SES-1251 - App crash on non alphanumeric first char search (#1393)

* Investigation in progress

* Working fix push before cleanup

* Fixes signalapp#1346

* Removed unused logging imports

* Put back some whitespace

* Minor cleanup

* Push before cleanup

* Fixes signalapp#1346 - properly this time!

* SES1567 - Community message delivery status fix (#1442)

* Initial investigation

* WIP

* Continued work to track down cause of delivery status issue

* Fixes signalapp#1438

* Cleanup for PR

* Further cleanup

* Fixed merge conflict

* Addressed PR feedback

---------

Co-authored-by: alansley <[email protected]>

* Tiny adjustment to center user name in Settings activity (#1446)

* Addressed PR feedback

* Cleanup

* Initial fix implemented

* Fixes signalapp#1448

* Addressed PR feedback

* SES1688 - Deleting last message in conversation, group, or community leaves the RecyclerView in a broken state (#1449)

* Initial fix implemented

* Fixes signalapp#1448

* Addressed PR feedback

* Handle case where there are no messages

* build: update build number

* Fix spacing when title is absent

* Hide reply button in MessageDetails for group invitations

* Remove reply from context menu for open group invitations

* Ignore swipe reply to open group invitation

* Fix multiple quote previews

* Fix message menu icons not visible in light theme

* Hide reply app bar menu item for open group invite

* SES-1727 Mentions text is the wrong colour (#1454)

* Fixes signalapp#1453

* Cleanup

* Code review adjustments

* Adjusted mentions to use the accent colour as their background colour when using light themes

---------

Co-authored-by: alansley <[email protected]>

* Disable swipe to reply on open group invites

* Fix multiple link previews

* SES1718 - Message Sending Status (#1462)

* Investigation in progress

* Initial push for PR

* Fixes signalapp#1461

* Removed leftover debug comments

* Added minor optimisation to showMessageStatus method (bail early if the message isn't one we care about displaying details of to the user)

* Minor cleanup

* Tiny cleanup

* Addressed PR feedback

* Removed forgotten debug log line & forced delivery status elements to be removed on non-visible messages just in case

* Minor refactor to simplify 'VisibleMessageView.showStatusMessage'

---------

Co-authored-by: alansley <[email protected]>

* Fix margins

* WIP

* Commit before converting SmsDatabase from Java to Kotlin

* Remove old expiration config strings from UpdateMessageBuilder

* Fix group expiration update config messages

* Fixed conversation view closing + hopefully wrong status text displayed + deletion of contact on removal of last message in 1-on-1 convo

* Cleanup for PR review

* Implemented PR feedback

* Don't start expiration for group expiration update messages

* Fix expiry update message for groups

* Correctly don't start disappear timer on group timer updates

* SES1813 - Fix regression test failures (#1473)

* Initial fix for regression test failure 1.1

* Added permissions fix for sharing documents which should allow for thumbnail generation

* Minor touch-up prior to merge into dev

* Fixes signalapp#1813

* Fixes signalapp#1472 - please ignore previous fixes 1813 statement, I'd used the Jira ticket number rather than creating a GitHub issue and using that

---------

Co-authored-by: alansley <[email protected]>

---------

Co-authored-by: alansley <[email protected]>
Co-authored-by: 0x330a <[email protected]>
Co-authored-by: Al Lansley <[email protected]>
Co-authored-by: AL-Session <[email protected]>
Co-authored-by: Al Lansley <[email protected]>
@AL-Session AL-Session deleted the SA1416_BanAndDeleteFunctionalityFix branch July 22, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0x330a 0x330a 0x330a approved these changes

Assignees
No one assigned
Labels
qa-tested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AL-Session @0x330a @burtonemily @alansley