PREAPPS-3593: Folder count issue fixed for shared folder. #278
Conversation
anukritiJagatramka
requested review from
BhavinBathani,
itsmeprashant,
pl12133,
silentsakky and
yashrajpchavda
as code owners
src/schema/notifications.ts
Outdated
| @@ -302,8 +322,12 @@ export class ZimbraNotifications { | |||
| if (items) { | |||
| items.forEach((i: any) => { | |||
| const item = normalizeFolder(i); | |||
| let itemId: any; | |||
There was a problem hiding this comment.
There was another PR - #270. Aren't we proceeding with that ?
There was a problem hiding this comment.
The approch was changed, incorporated the same.
src/schema/notifications.ts
Outdated
| f.includes('Folder:') | ||
| ); | ||
| const idSplit = item.id.split(':'); | ||
| for (let folderId in allFolders) { |
src/schema/notifications.ts
Outdated
| cachedData[allFolders[folderId]].ownerZimbraId === idSplit[0] && | ||
| cachedData[allFolders[folderId]].sharedItemId === idSplit[1] | ||
| ) { | ||
| sharedFolderId = cachedData[allFolders[folderId]].id; |
There was a problem hiding this comment.
Might want to explain break condition in comment.
There was a problem hiding this comment.
sharedFolderId = allFolders[folderId][id]
There was a problem hiding this comment.
@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.
src/schema/notifications.ts
Outdated
| const idSplit = item.id.split(':'); | ||
| for (let folderId in allFolders) { | ||
| if ( | ||
| cachedData[allFolders[folderId]].ownerZimbraId === idSplit[0] && |
There was a problem hiding this comment.
Store cachedData[allFolders[folderId]] into a variable and use it at multiple places.
There was a problem hiding this comment.
allFolders[folderId][ownerZimbraId] holds same data as cachedData[allFolders[folderId]].ownerZimbraId
There was a problem hiding this comment.
@itsmeprashant As mentioned above, allFolders is an array of string of type Folder:.
@avadhutprabhudesai Stored the data in variable and used it everywhere.
| @@ -145,6 +145,26 @@ export class ZimbraNotifications { | |||
| this.getApolloClient().queryManager.broadcastQueries(); | |||
| }; | |||
|
|
|||
| // Find the actual folder of the shared folder | |||
| private findSharedItemId = (item: any) => { | |||
There was a problem hiding this comment.
Can we destructure id here?
There was a problem hiding this comment.
Tweaked logic to only send id to findSharedItemId, hence destructuring will not be required.
src/schema/notifications.ts
Outdated
| @@ -302,8 +322,12 @@ export class ZimbraNotifications { | |||
| if (items) { | |||
| items.forEach((i: any) => { | |||
| const item = normalizeFolder(i); | |||
| let itemId = item.id; | |||
| if (i.id.includes(':')) { | |||
| itemId = this.findSharedItemId(i); | |||
There was a problem hiding this comment.
Can we just pass id to this function? Do we need any other attributes from item?
src/schema/notifications.ts
Outdated
| @@ -302,8 +322,12 @@ export class ZimbraNotifications { | |||
| if (items) { | |||
| items.forEach((i: any) => { | |||
| const item = normalizeFolder(i); | |||
| let itemId = item.id; | |||
There was a problem hiding this comment.
const itemId = i.id.includes(':') ? this.findSharedItemId(i.id) : item.id; ?
src/schema/notifications.ts
Outdated
| @@ -302,8 +322,12 @@ export class ZimbraNotifications { | |||
| if (items) { | |||
| items.forEach((i: any) => { | |||
| const item = normalizeFolder(i); | |||
| let itemId = item.id; | |||
| if (i.id.includes(':')) { | |||
There was a problem hiding this comment.
get(i, 'id', '').includes(':')
src/schema/notifications.ts
Outdated
| folder.sharedItemId === idSplit[1] | ||
| ) { | ||
| sharedFolderId = folder.id; | ||
| break; |
There was a problem hiding this comment.
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.
src/schema/notifications.ts
Outdated
| @@ -302,8 +324,11 @@ export class ZimbraNotifications { | |||
| if (items) { | |||
| items.forEach((i: any) => { | |||
| const item = normalizeFolder(i); | |||
| const itemId = get(i, 'id', '').includes(':') | |||
There was a problem hiding this comment.
We can access item.id here safely instead of using get, and it is confusing to use i instead of item.
No description provided.