the email match should be case insensitive when selecting the alias #3473
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: This commit makes it so `resetEmailCache` works as expected, i.e. it removes all databases, without forcing the user to re sign-in to their accounts or NylasID Previously, this method removed the database without removing the accounts, left users in an un-authed state that was hard to recover from. This was fixed in D4212 which makes sure that when we get a new identity, sync and deltas are restarted However, resetEmailCache would still force you to log in to yoru NylasID because it was deleted from the database. However, if we reuse the command `application:relaunch-to-initial-windows` instead of manually deleting the database, we can relaunc the app while preserving the users NylasID session, so they don't have to sign back in manually. Test Plan: manual Reviewers: evan, spang, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4215
Summary: Use electron's `powerMonitor` module to detect when the computer resumes from sleep, and restart the sync loop when that happens in order to sync the inbox immediately, in case we received any new mail events while the computer was asleep Test Plan: manual Reviewers: evan, spang, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4216
Summary: On windows running `git rev-parse --short HEAD` does in fact now give you 9 characters instead of 7 like it does on Mac. This will ensure that builds get uploaded to the same folder and help ensure we don't post a version that doesn't exist on the release page. Test Plan: Manual Reviewers: juan, halla, spang Reviewed By: spang Differential Revision: https://phab.nylas.com/D4217
Summary: Minor copy change via Sachin Test Plan: manual Reviewers: juan, spang, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4218
Summary: This is going to be a diff way shorter than the previous one! Basically, it adds a new endpoint, `/blobs` to our API to store send later attachments. When a user schedules a draft to be sent, we send all attachments to this endpoint. Separately, we store the rest of the message as metadata. When it's time to send the message, we fetch the attachments from S3, fetch the metadata and merge them together to get a message we can send. Test Plan: Tested manually. Will make a final QA pass before landing. Reviewers: juan, halla, evan Reviewed By: halla, evan Differential Revision: https://phab.nylas.com/D4196
Summary: We need to upload the nupkg for the Windows autoupdater to work Test Plan: manual Reviewers: juan, spang, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4219
Summary: The ignore list was very old. It included several dozen MB of docs_src and other crap in our builds Test Plan: manual Reviewers: halla, spang, juan Reviewed By: juan Differential Revision: https://phab.nylas.com/D4213
Summary: We don't need to spend the time running script/bootstrap in our cloud-* folders! Test Plan: manual Reviewers: spang, juan, halla Reviewed By: juan, halla Differential Revision: https://phab.nylas.com/D4214
Summary:
We were using a version that was ~9 months old and a lot of development has
happened since.
v3 is not compatible w/v2, but it looks like we aren't using any of the
features that had breaking changes:
http://nodemailer.com/about/migrate/
I chose not to switch to the new built-in OAuth2 token refresh support
because we already have a mechanism for refreshing oauth tokens and
adding a different implementation specifically for SMTP would introduce
more opportunities for bugs.
Since mailcomposer is no longer a dependency of nodemailer, I added this
dependency as well.
Test Plan: manual - sent a message, sent a message w/an attachment
Reviewers: evan, khamidou, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4201
…y received Summary: We don't want to bump threads to the top of the inbox when a user sends a reply. We originally used `!isSent` to prevent this, but that was removed in a diff that made sure messages showed up in the inbox when users send emails to themselves. In order to implement both of these cases properly, this diff introduces `isReceived` and uses that to determine whether lastReceivedDate should be updated. Addresses T7991. Also changes the order of some `or` statements, so that we actually check that the variable exists before comparing against it. Test Plan: manual Reviewers: evan, juan, spang Reviewed By: spang Differential Revision: https://phab.nylas.com/D4226
Summary: Previously, after creating a new folder, the UI would indicate that the new folder had children, even though it didn't. This was caused by duplicate models in our `MutableQueryResultSet` for the user's categories. Basically, we would sync the server version of the folder before the `SyncbackTask` for the new folder returned its `serverId`. Without the `serverId`, the synced version of the folder couldn't yet be tied to the optimistic folder, so a second row was created in the database. This second row is removed when the `syncbackTask` does return the `serverId`, because we persist the optimistic folder with a `REPLACE INTO` query. (This deletes other rows with the same id.) However, since this was done inside a `persist` change with the `serverId` and no `unpersist` was ever recorded for the `clientId`, our `MutableQueryResultSet` never removed the `clientId` model. To address this, this diff adds a check in `updateModel` to see if the `serverId` is being added. If it is, and both the `serverId` and `clientId` exist in the `_ids` list, we remove the `clientId`. The children indicator does still briefly show up while there are still two separate rows for that folder in the database. If we want to get rid of this completely, we would have to ensure that we do not sync the folder before the `syncbackTask` returns the `serverId`. However, this would probably be pretty involved, and for not much gain. This fix is much simpler and reduces most of the issue. Test Plan: manual Reviewers: juan, evan Reviewed By: evan Differential Revision: https://phab.nylas.com/D4228
This reverts commit e798e50. Reason for revert: breaks Send Later. Needs more testing.
Summary: see title. also convert to es6 Test Plan: manual Reviewers: evan, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4225
for ei-prod only. I set up the ei EB env as prod level, but have been working on launch changed in master branch. Easy enough to undo, but I didn't wan't to mess with prod branch until done because the launch scripts affect all the n1cloud-related environments.
Summary: When querying transactions for the delta stream, if no cursor is provided, we should default to 0. Otherwise this will generate a query like: ``` WHERE `transaction`.`id` > ‘null’ ``` Which is obviously wrong Test Plan: manual Reviewers: evan, halla, spang Reviewed By: spang Differential Revision: https://phab.nylas.com/D4242
Summary:
Currently, when we auth an account for the first time in Nylas Mail (or
we blow away the database), the app is going to request transactions
since cursor `null` from the /delta/streaming endpoint and from the local-sync
delta observable, instead of requesting transactions since cursor `0`
This is due to a subtle bug with the use of default values when
destructuring an object. Our coded did the following:
```
const {cursor = 0} = this._state
```
Which at a glance seems correct. However, this will only work as
expected if `this._state` has the following shape:
```
{cursor: undefined}
```
And unfortunately, our `this._state` looked like this when authing an
account for the first time:
```
{cursor: null}
```
Which would make `cursor === null` instead of `0`.
This is because when using default values, null is considered an
intentional argument/value, as opposed to not passing any argument/value
(which will mean that the argument is undefined).
This was a regression introduced in d60a23c and 8bc2ec5
Test Plan: manual, will add regression test in upcoming diff
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4243
Summary: This was showing up in Sentry. Check to see if the recipient has an email before calling toLowerCase. Test Plan: yolo Reviewers: halla, juan Reviewed By: juan Differential Revision: https://phab.nylas.com/D4423
Summary: By nullifying the key inside `workersByAccountId` we would attempt to access accountIds that no longer existed in other parts of the code by using Object.keys() This makes it so we correctly completely remove the record from the map Test Plan: manual Reviewers: halla, spang, mark Reviewed By: mark Differential Revision: https://phab.nylas.com/D4426
Summary: After https://github.com/nylas/nylas-mail-all/commit/008cb4c, the shape of account.connectionSettings changed, which means that ids for accounts will also change, given that they are based on the connection settings (see Account.hash()). This is fine in most cases, except for accounts that were running on a version of Nylas Mail before 008cb4c, then upgraded to a version after 008cb4c, and then re-authed their account. In this scenario, re-authing the account will create a second account with a different `id` but with the same email address, along with an extra sequelize database, and will start a second SyncWorker for the same account. App-side, the `AccountStore` will correctly overwrite the first account, so users would correctly see just 1 account in the sidebar. However, given that 2 SyncWorkers would be running for the same account, message ids would collide in edgehill.db and this would cause threads to disappear from the ui as if they were being deleted. To fix this, we need to do 2 things: - Upon app start, we remove any duplicate accounts that might have been created due to this bug before starting any sync workers - If we detect that we are going to create a duplicate account upon auth success, we delete the old account first. This will effectively cause sync to restart for the account. Test Plan: Verified problem: Checked out a commit before 008cb4c, authed an account, checked out master, re-authed account, verified that duplicate accounts are created. Then test 2 scenarios: - With duplicate accounts present, checked out this commit, verified that duplicate account would be removed and sync would function normally. - With an account authed on a build before 008cb4c, checked out this commit, re-authed the account, and verified that duplicate account wouldn't be created Reviewers: mark, halla, khamidou, spang Reviewed By: spang Subscribers: tomasz Differential Revision: https://phab.nylas.com/D4425
Summary: Which was arguably already a code smell being inside the DatabaseStore Test Plan: manual Reviewers: mark, halla, evan Reviewed By: evan Differential Revision: https://phab.nylas.com/D4430
Summary: This commit addressess issue T8135, which prevents the app from starting. This was happening because when 3111c16 landed, we added database access from the main (backend) electron process to be able to read the identity now stored in the database: nylas@3111c166#diff-1efa26fa0ae1603366b2c0033d971028R44 However, we omitted to add any error handling, so if the database failed to open due to a database malformed error (which it does: https://sentry.io/nylas/nylas-mail/?query=is%3Aunresolved+release%3A2.0.14+malformed&statsPeriod=14d), the app will just fail to start, given that this happens during the initialization of the main process. Additionally, the fact that we had no error handling increased the error reports for malformed errors given that we would never handle them, so every-time we opened the app we would report the same error This commit adds the same error handling we have in the DatabaseStore and moves the code around so it's available both in the main and renderer processes. After this commit, if the database fails to open during main process initialization, due to malformed errors or others, we will correctly inform the user that the database is corrupted, rebuild it, and restart the app. Test Plan: manually throw errors during setup, verify that we handle them correctly Reviewers: mark, spang, evan, halla Reviewed By: evan, halla Differential Revision: https://phab.nylas.com/D4431
Summary: see title Test Plan: manual Reviewers: halla, mark, evan Reviewed By: evan Differential Revision: https://phab.nylas.com/D4432
Summary: Given that this error is unrecoverable it shouldn't be retryable. Additionally, this commit improves error handling for this error in a few ways: - Don't delete all databases, just edgehill.db which is the one we know is corrupted. (Use `handleDatabaseUnrecoverableError` instead of `Actions.resetEmailCache`) - Message the user about the database being reset so the app doesn't restart without notice, and make sure that this message is only displayed once by moving it to the main process Test Plan: manual Reviewers: mark, spang, evan, halla Reviewed By: evan, halla Differential Revision: https://phab.nylas.com/D4433
Summary: We weren't passing benchmarkMode through in the default window options, so the worker window (or any other spawned window) would think it wasn't in benchmark mode. Test Plan: Run locally, verify that the worker window thinks it's in benchmark mode Reviewers: halla, spang, juan Reviewed By: juan Differential Revision: https://phab.nylas.com/D4427
…t files Summary: Previously we stored message bodies uncompressed inside two different databases. This was bad for a few reasons: * Duplicate data in multiple places is an obvious waste of disk space * Uncompressed data also made the disk footprint bigger than it might otherwise be * Storing these large message bodies in the database made the db file larger than it otherwise could have been, increasing the size of tables and slowing down queries. This diff adds support for storing message bodies outside of the database in compressed flat files. It changes the use of the body column in the K2 database and the MessageBody table in the edgehill database to contain a blob of JSON that contains a path to the file on disk. We use the new format in an incremental fashion without having to perform an actual database migration by first checking if the body matches our expected JSON format and treating it appropriately if it doesn't. Both databases refer to the same file on disk, thus deduplicating the messages bodies. We also transparently support gzipping the message bodies stored on disk when we read from/write to the files. The real world space savings depends on the compressibility of the messages, but we've seen up to ~60% improvement in disk space usage for certain inboxes. Typical savings are closer to 20%. Also, by storing messages as separate files on disk we can potentially integrate with Spotlight search at some point in the future. Test Plan: Run locally, make sure that upgrade to this doesn't hose things, look at size of DB, read emails Reviewers: evan, spang, halla, juan Reviewed By: halla, juan Differential Revision: https://phab.nylas.com/D4422
Summary: see title Test Plan: manual Reviewers: spang, evan, mark, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4445
Summary: see title Test Plan: manual Reviewers: evan, spang, mark, halla Reviewed By: halla Differential Revision: https://phab.nylas.com/D4446
Summary: see title Test Plan: manual Reviewers: mark Differential Revision: https://phab.nylas.com/D4447
The _fromContactForReply selects the right email address to fill the from field when replying to messages. The emailMatch should be case insensitive when comparing the list of alias and the to field of the email.
This was referenced Apr 24, 2017
|
@yehiasalam I know this PR is from some time ago. Nylas is no longer supporting this product, but we would love to have a PR for this against Nylas Mail Lives. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The _fromContactForReply selects the right email address to fill the from field when replying to messages. The emailMatch should be case insensitive when comparing the list of alias and the to field of the email. This solves the issue for ticket #3462 and #3454