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

Add Empty Spam command #8126

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

Conversation

polyzen
Copy link

@polyzen polyzen commented Sep 6, 2024

Closes #5829

Not tested yet, just copy and pasted what I found of the "Empty Trash" command.
Seems legacy/core/src/test/java/com/fsck/k9/controller/PendingCommandSerializerTest.java can be left as-is?

@polyzen polyzen changed the title Add Empty Trash command Add Empty Spam command Sep 6, 2024
@cketti
Copy link
Member

cketti commented Oct 15, 2024

Thanks for the pull request! At a first glance it looks good to me 👍

Sorry, for not providing any feedback earlier. You have probably noticed that we're very busy with getting ready for the first release of Thunderbird for Android. We don't want to include new unplanned features before the release. However, we'll get back to this after the initial release of Thunderbird for Android.

Copy link
Member

@cketti cketti left a comment

Choose a reason for hiding this comment

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

Unfortunately, this does need some more work. Currently the code doesn't even compile. Please address the review comments and make sure to actually build and test your change on a device or emulator.

Comment on lines +2119 to +2125
boolean isSpamLocalOnly = isSpamLocalOnly(account);
if (isSpamLocalOnly) {
localFolder.clearAllMessages();
} else {
localFolder.destroyLocalOnlyMessages();
localFolder.setFlags(Collections.singleton(Flag.DELETED), true);
}
Copy link
Member

Choose a reason for hiding this comment

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

Unlike with the trash folder, there never is a local-only spam folder. Assume isSpamLocalOnly is false and simplify the if statement, i.e. only keep the code in the else block.

Comment on lines +796 to +797
val title = getString(R.string.dialog_confirm_empty_spam_title)
val message = getString(R.string.dialog_confirm_empty_spam_message)
Copy link
Member

Choose a reason for hiding this comment

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

These strings are missing.

@@ -738,6 +738,12 @@ class MessageListFragment :
}
}

private fun onEmptySpam() {
if (isShowingSpamFolder) {
showDialog(R.id.dialog_confirm_empty_spam)
Copy link
Member

Choose a reason for hiding this comment

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

The ID needs to be added to legacy/ui/legacy/src/main/res/values/ids.xml

@@ -738,6 +738,12 @@ class MessageListFragment :
}
}

private fun onEmptySpam() {
if (isShowingSpamFolder) {
Copy link
Member

Choose a reason for hiding this comment

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

The isShowingSpamFolder property doesn't exist.

Copy link
Member

Choose a reason for hiding this comment

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

MessagingController already is way too large. Please move your new code to SpamOperations and write it in Kotlin. See DraftOperations for an example.

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 this pull request may close these issues.

Add "Empty Spam" action for the Spam folder
2 participants