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.

Sign up for GitHub

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

Jump to bottom

PREAPPS-3593: Folder count issue fixed for shared folder. #278

Merged
merged 4 commits into from
Nov 5, 2019
Merged

PREAPPS-3593: Folder count issue fixed for shared folder. #278

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9a3b9b8
PREAPPS-3593: Folder count issue fixed for shared folder.
Oct 18, 2019
a0d1cab
PREAPPS-3593: Testcases failure fixed.
Oct 18, 2019
a31ae9c
PREAPPS-3593: Code Improvements.
Oct 23, 2019
73d5d98
PREAPPS-3593: Code Improvements.
Oct 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/schema/notifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,26 @@ export class ZimbraNotifications {
this.getApolloClient().queryManager.broadcastQueries();
};

// Find the actual folder of the shared folder
private findSharedItemId = (item: any) => {
Copy link

Choose a reason for hiding this comment

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

Can we destructure id here?

Copy link
Author

Choose a reason for hiding this comment

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

Tweaked logic to only send id to findSharedItemId, hence destructuring will not be required.

let sharedFolderId: any;
const cachedData = get(this.cache, 'data.data');
const allFolders = Object.keys(cachedData).filter(f =>
f.includes('Folder:')
);
const idSplit = item.id.split(':');
for (let folderId in allFolders) {
Copy link

Choose a reason for hiding this comment

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

can folderId be const?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

if (
cachedData[allFolders[folderId]].ownerZimbraId === idSplit[0] &&
Copy link

Choose a reason for hiding this comment

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

Store cachedData[allFolders[folderId]] into a variable and use it at multiple places.

Copy link
Contributor

Choose a reason for hiding this comment

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

allFolders[folderId][ownerZimbraId] holds same data as cachedData[allFolders[folderId]].ownerZimbraId

Copy link
Author

Choose a reason for hiding this comment

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

@itsmeprashant As mentioned above, allFolders is an array of string of type Folder:.
@avadhutprabhudesai Stored the data in variable and used it everywhere.

cachedData[allFolders[folderId]].sharedItemId === idSplit[1]
) {
sharedFolderId = cachedData[allFolders[folderId]].id;
Copy link

Choose a reason for hiding this comment

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

Might want to explain break condition in comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

sharedFolderId = allFolders[folderId][id]

Copy link
Author

Choose a reason for hiding this comment

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

@itsmeprashant allFolders is an array of keys of type Folder:, for example - ['Folder:2', 'Folder:1999']. It is different from cached data.
@avadhutprabhudesai Added comment.

break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing an assignment and a break here, you can instead return folder.id; and won't need to break out of the loop because the function ends.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

}
}
return sharedFolderId;
};

private handleContactNotifications = (notification: Notification) => {
const items = itemsForKey(notification, 'cn');
this.batchProcessItems(items, this.processContactNotifications);
Expand Down Expand Up @@ -302,8 +322,12 @@ export class ZimbraNotifications {
if (items) {
items.forEach((i: any) => {
const item = normalizeFolder(i);
let itemId: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

There was another PR - #270. Aren't we proceeding with that ?

Copy link
Author

Choose a reason for hiding this comment

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

The approch was changed, incorporated the same.

if (i.id.includes(':')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

get(i, 'id', '').includes(':')

Copy link
Author

Choose a reason for hiding this comment

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

Done.

itemId = this.findSharedItemId(i);
Copy link

Choose a reason for hiding this comment

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

Can we just pass id to this function? Do we need any other attributes from item?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

}
this.cache.writeFragment({
id: `Folder:${item.id}`,
id: `Folder:${itemId}`,
fragment: gql`
fragment ${generateFragmentName('folderNotification', item.id)} on Folder {
${attributeKeys(item)}
Expand Down