feature: Link notes to a mail and show them in the mail view - #101
Open
r4nc0r wants to merge 6 commits into
Open
feature: Link notes to a mail and show them in the mail view#101r4nc0r wants to merge 6 commits into
r4nc0r wants to merge 6 commits into
Conversation
Contributor
Author
|
do you have any feedback on this? |
Member
I'm going through the github PRs today, so I'll comment if I have some feedback. |
Member
|
First I have some general notes:
|
Contributor
Author
|
all done |
This file contains hidden or 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
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.
Lets a sticky note be linked to a mail and shown in the mail header, so that a mail can be annotated without touching it.
The note holds the mail's
PR_INTERNET_MESSAGE_IDin anote_link_idnamed property, and is created in the Notes folder of the store the mail lives in rather than the user's own, so everyone with access to that mailbox sees the same notes. Nothing is written to the mail:A mail with no
PR_INTERNET_MESSAGE_IDcannot be annotated. Those are not received mail, which is what this is for.The panel hangs off the
previewpanel.toolbar.detaillinksinsertion point next to the calendar's meeting info, so it appears in the preview panel, the separate window and on conversation cards alike. It renders nothing for a mail without notes. Note bodies are plain text and are escaped before display. The note list now also fetchesPR_BODYso a note can be read where it is shown rather than only after opening it; sticky notes are small and live in a single folder, so unlike the mail list this does not meaningfully grow the request.Notes are added from the mail context menu. The note dialog names the mail a note is on and opens it; for that direction the note also records the mail's entryid, parent entryid and subject. A mail is identified by all of its id properties together, so all of them are kept: a record built from a partial id is compared against any record already loaded for the same mail while it opens, and that comparison then dereferences null, leaving a tab that never finishes loading. A record already loaded for the mail is reused in preference to building a second one.
The named properties are keyed by string rather than dispid, since the id space is shared and a number could collide.
The property cache fix
The first commit fixes a bug in
Propertiesthat this feature cannot work on a shared mailbox without, kept separate so it can be reviewed and reverted on its own.Propertiescaches resolved named property ids underPR_MAPPING_SIGNATURE, so that each store gets its own set. gromox reports an all-zeroPR_MAPPING_SIGNATUREfor every store while still allocating named property ids per store, so every store shares one cache entry and the first store to resolve a name hands its ids to all the others.getStoreMappingSignature()now falls back to the store entryid, which does identify the store.The key also has to be recomputed when the active store changes.
Init()only does that while$initis false, and$initis reset once per request rather than once per module, so in a request touching several stores the second module onwards kept answering for the first module's store.setStore()now clears$init. Either change alone is insufficient.This is not noticeable on the named properties in the tree today, because they happen to resolve to the same ids in every store: they are allocated in the same order in each freshly created store. It becomes noticeable as soon as a named property is introduced that stores allocate at different times, since each store then has its own allocation history —
note_link_idresolved to0x82C9in one store and0x82C3in another, and the cache handed the first store's id to both.Testing
Tested against a shared mailbox with two users: one with read-only access to the other's Inbox and write access to its Notes folder, confirming the annotation is written into the shared store and is visible to the mailbox owner on the same mail, and that the mail's
PR_BODYandPR_LAST_MODIFICATION_TIMEare unchanged.