SimpleMobileTools / Simple-SMS-Messenger

An easy and quick way of managing SMS and MMS messages without ads.
https://www.simplemobiletools.com
GNU General Public License v3.0
600 stars 218 forks source link

Add support for recycle bin for messages #683

Closed esensar closed 1 year ago

esensar commented 1 year ago

Archiving messages currently acts like recycle bin in Simple Gallery, meaning that after 30 days, messages will be deleted permanently.

This closes #451

NOTE: This is still a work in progress, many strings are wrong or missing and it is not completely tested.

tibbi commented 1 year ago

how does it work? I have the setting enabled but it still deletes everything instantly. Also lets change the section label to "Archive" and Im thinking of disabling it by default too, will see. I never needed archive and it might not be needed by most casual people.

tibbi commented 1 year ago

oh I see now... Archive should apply to individual messages too, not just whole conversations.

esensar commented 1 year ago

How should it work in that case? Where would the user access the archived messages? I considered that, but couldn't figure out some sensible UX, so I limited it to conversations for now.

tibbi commented 1 year ago

It should basically show the same way as on the main screen. If there is an archived message with a given contact, we show the given conversation first with the latest archived message snippet. The user has to then open it to see individual archived messages.

esensar commented 1 year ago

It should basically show the same way as on the main screen. If there is an archived message with a given contact, we show the given conversation first with the latest archived message snippet. The user has to then open it to see individual archived messages.

I think that would make the feature much more complex. Looking at the original issue (#177), seems like most people are interested in archiving only conversations. Should we do it in stages, to see if there is a need to support archiving individual messages?

tibbi commented 1 year ago

well, I dont really see much sense in archiving whole conversations only. I was thinking about this more like a recycle bin for messages, thats what makes sense. So I think we should do it properley now with message archiving too, not step by step as usually.

tibbi commented 1 year ago

change the UX a bit too, make it consistent with the gallery. After long pressing a message/conversation, do not show both a trashbin and Archive menu items, just a trashbin. If archiving is enabled in the app settings, it will archive a message. If it is disabled, it will delete it instantly. Confirmation dialogs will change depending on that too.

esensar commented 1 year ago

change the UX a bit too, make it consistent with the gallery. After long pressing a message/conversation, do not show both a trashbin and Archive menu items, just a trashbin. If archiving is enabled in the app settings, it will archive a message. If it is disabled, it will delete it instantly. Confirmation dialogs will change depending on that too.

Not sure if I am missing something, but that is how I implemented it right now. There is no additional menu item for archiving, only trash bin is present at all times and depending on archive setting, dialog text changes and also if archive is enabled, the confirmation dialog has additional checkbox to skip archive (similar to recycle bin in gallery).

tibbi commented 1 year ago

alright, I thought that I saw such a menu item, maybe Im wrong then

esensar commented 1 year ago

I have implemented archiving of individual messages. It is a bit messy right now, but if the current behavior and experience is looking alright, then I can move on to cleaning it up a bit.

Since I had to change the database a bit compared to previous PR state, you will have to remove the data, or update from master branch version of the app.

tibbi commented 1 year ago

can you check the conflict please

tibbi commented 1 year ago

clean install is crashing on android 13 11:35:21.093 E FATAL EXCEPTION: main Process: com.simplemobiletools.smsmessenger.debug, PID: 8523 java.lang.RuntimeException: Unable to create application com.simplemobiletools.smsmessenger.App: java.lang.SecurityException: getGroupIdLevel1 at android.app.ActivityThread.handleBindApplication(ActivityThread.java:7619) at android.app.ActivityThread.-$$Nest$mhandleBindApplication(Unknown Source:0) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2400) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loopOnce(Looper.java:226) at android.os.Looper.loop(Looper.java:313) at android.app.ActivityThread.main(ActivityThread.java:8757) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:604) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067) Caused by: java.lang.SecurityException: getGroupIdLevel1 at android.os.Parcel.createExceptionOrNull(Parcel.java:3023) at android.os.Parcel.createException(Parcel.java:3007) at android.os.Parcel.readException(Parcel.java:2990) at android.os.Parcel.readException(Parcel.java:2932) at com.android.internal.telephony.IPhoneSubInfo$Stub$Proxy.getGroupIdLevel1ForSubscriber(IPhoneSubInfo.java:1328) at android.telephony.TelephonyManager.getGroupIdLevel1(TelephonyManager.java:5704) at android.telephony.TelephonyManager.getMmsUserAgent(TelephonyManager.java:7816) at com.klinker.android.send_message.ApnUtils.loadMmsSettings(ApnUtils.java:172) at com.klinker.android.send_message.ApnUtils.initDefaultApns(ApnUtils.java:28) at com.simplemobiletools.smsmessenger.App.onCreate(App.kt:14) at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1266) at android.app.ActivityThread.handleBindApplication(ActivityThread.java:7614) at android.app.ActivityThread.-$$Nest$mhandleBindApplication(Unknown Source:0)  at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2400)  at android.os.Handler.dispatchMessage(Handler.java:106)  at android.os.Looper.loopOnce(Looper.java:226)  at android.os.Looper.loop(Looper.java:313)  at android.app.ActivityThread.main(ActivityThread.java:8757)  at java.lang.reflect.Method.invoke(Native Method)  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:604)  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067) 

esensar commented 1 year ago

This seems to be related to work done in https://github.com/SimpleMobileTools/Simple-SMS-Messenger/pull/687 It is related to the first changes, which haven't really fixed the issue, so I will open up a PR to remove these changes, to prevent this crash from occurring. It seems that newer Android versions have completely blocked off access to APN info.

esensar commented 1 year ago

https://github.com/SimpleMobileTools/Simple-SMS-Messenger/pull/695 This PR should resolve that crash

tibbi commented 1 year ago

and what does missing APN info cause? Something wont work?

naveensingh commented 1 year ago

Came here from the MMS duplication PR, isn't there an ARCHIVED column in the Android Threads table that we can use to archive threads or do we want to handle everything ourselves?

and what does missing APN info cause? Something wont work?

It won't break anything that wasn't already broken. Like before, we can't send ack msg to the MMS server without this MMSC info.

tibbi commented 1 year ago

it would definitely be better to use a built in Archived field, if it is reliable and available in all versions

naveensingh commented 1 year ago

It's only available for Threads (API 21+), messages are just deleted. I haven't checked if other messaging apps respect or utilize the ARCHIVED column but they probably do.

tibbi commented 1 year ago

21+ is obviously enough as our min is 23

esensar commented 1 year ago

So is the decision to use ARCHIVED column of Threads instead of this then?

If we decide to use that, then this will be an actual archive functionality and not recycle bin as you initially wanted. It will also not support archiving individual messages. Just wanted to confirm if this is the way we want to go.

tibbi commented 1 year ago

oh right, if you cannot archive individual messages, Im not interested in it. Lets keep the current own solution then. Will test it soon.

tibbi commented 1 year ago

lets remove "Mark as unread" from archived menus, doesnt make sense having it there. Now that Im looking at it though, we might indeed split archive and trashbin later :/ This is not the behaviour people are describing in the given issue.

esensar commented 1 year ago

Before making further changes, I think we need to decide if this needs to have that recycle bin behavior, which should represent messages scheduled to be deleted, or archive in a sense that they are moved out of the main view. Depending on that, we can more easily decide other stuff, like options in menus and similar things

naveensingh commented 1 year ago

Indeed. Archived and Recycle Bin are two different things. Archived implies the conversations will be always there, just not actively interacted with, and can be unarchived whenever needed. Recycle Bin implies it's non-interactive and is deleted after some specified time period.

@tibbi I think we should implement the Archive option (managed in-app or system based). To me, the idea of a recycle bin is appealing only in the case of individual messages. We could do Archive for conversations only and Recycle bin for messages.

tibbi commented 1 year ago

Sadly I never needed neither a trashbin, nor archive in an SMS Messenger, so got a bit confused about what is it about at all... Lets change this PR to recycle bin functionality and implement archive as a separate PR, by using the Archived flag at threads only.

esensar commented 1 year ago

Sounds good. I will get the conversation archive feature ready first, since it is kind of straightforward, whereas recycle bin for messages will probably need some more work.

esensar commented 1 year ago

Since this PR has become a bit messy, I will close it and open a new one for recycle bin for messages.

698 contains the archiving feature for threads.

esensar commented 1 year ago

https://github.com/SimpleMobileTools/Simple-SMS-Messenger/pull/700 is a WIP PR for recycle bin for messages, discussion can continue there regarding that feature