Skip to content

feat: remove gmail prefix in folder structure #9213

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

Merged
merged 1 commit into from
Jun 6, 2025

Conversation

shamim-emon
Copy link
Collaborator

@shamim-emon shamim-emon force-pushed the fix-issue-9134 branch 4 times, most recently from 9d74c82 to 8a2759c Compare May 25, 2025 12:26
Copy link
Contributor

@asoucar asoucar left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

@kewisch
Copy link
Member

kewisch commented May 28, 2025

Is this something we could rather change in the folder modeling code to just skip the [Gmail] folder completely, rather than just formatting away the name?

Also note that sometimes the folder is called [Google Mail], and we should only be doing this on gmail based accounts.

@asoucar
Copy link
Contributor

asoucar commented May 28, 2025

Is this something we could rather change in the folder modeling code to just skip the [Gmail] folder completely, rather than just formatting away the name?

Also note that sometimes the folder is called [Google Mail], and we should only be doing this on gmail based accounts.

Its my (admittedly imperfect) understanding that the first section of code is doing the filtering out? Clearly it would need some additional robustness if the folder has alternate names

@shamim-emon shamim-emon force-pushed the fix-issue-9134 branch 2 times, most recently from c999e62 to d1b1a34 Compare May 31, 2025 18:35
@shamim-emon shamim-emon requested a review from asoucar May 31, 2025 19:49
@asoucar
Copy link
Contributor

asoucar commented Jun 2, 2025

Thank you for the update! While reviewing this again and thinking further about the problem, I think this is heading in the right direction but is starting with a fundamental gap. Right now this (and other current filtering ideas) is happening on the UI layer. Your fix would have the app attempting to update the database every time the MessageList screen is created. While the display name for a folder is a UI component, the issue is at its core an issue with the data model. That makes addressing it at the UI layer a band-aid at best.
I think your actual filter code is actually pretty comprehensive. What if we could apply it as the database was being populated? That way it only has to happen once (with perhaps a migration step for previously loaded folder structures), decoupling the data model from the UI for better separation of concerns. Potentially there would then have to be some handling for when new emails are fetched to ensure they end up in the appropriate folder too.

@shamim-emon shamim-emon force-pushed the fix-issue-9134 branch 2 times, most recently from 6a7a8cf to 9861c17 Compare June 3, 2025 08:02
@shamim-emon
Copy link
Collaborator Author

shamim-emon commented Jun 3, 2025

Thank you for the update! While reviewing this again and thinking further about the problem, I think this is heading in the right direction but is starting with a fundamental gap. Right now this (and other current filtering ideas) is happening on the UI layer. Your fix would have the app attempting to update the database every time the MessageList screen is created. While the display name for a folder is a UI component, the issue is at its core an issue with the data model. That makes addressing it at the UI layer a band-aid at best. I think your actual filter code is actually pretty comprehensive. What if we could apply it as the database was being populated? That way it only has to happen once (with perhaps a migration step for previously loaded folder structures), decoupling the data model from the UI for better separation of concerns. Potentially there would then have to be some handling for when new emails are fetched to ensure they end up in the appropriate folder too.

@asoucar with the latest approach I'm doing following:

  1. I'm removing Gmail/Google Mail prefix while folders are being created here
  2. Also to deal with previously loaded folders containing Gmail/Google Mail prefix I am calling removeGmailPrefixFromFolders() when instance of CreateFolderOperations is being created like this. Calling removeGmailPrefixFromFolders() over there ensures it's called once when the instance of CreateFolderOperations is being injected.

Please let me know what do you think of this approach? Is it alright and/or anything else needed?

@asoucar
Copy link
Contributor

asoucar commented Jun 4, 2025

Thanks @shamim-emon ! I think this is 90% of the way there. As it stands right now I believe this will query all db folders, whether or not it is a Gmail account. For efficiency we should only do the folder filtering for a gmail account. But otherwise I think this is a solid solution

@shamim-emon shamim-emon force-pushed the fix-issue-9134 branch 2 times, most recently from 4f4e651 to 5f4fd3a Compare June 4, 2025 18:14
Copy link
Contributor

@asoucar asoucar left a comment

Choose a reason for hiding this comment

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

Thank your work on this!

@asoucar asoucar merged commit 6610cbd into thunderbird:main Jun 6, 2025
3 checks passed
@thunderbird-botmobile thunderbird-botmobile bot added this to the Thunderbird 12 milestone Jun 6, 2025
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.

Special case Gmail and hide [Gmail] / [Google Mail] in the folder structure
3 participants