Skip to content

add depth check to oneNoteItemUtils #118

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
34 changes: 34 additions & 0 deletions src/oneNoteDataStructures/oneNoteItemUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { Polyfills } from '../polyfills';

Polyfills.find();

export type noteBookOrSectionGroup = Notebook | SectionGroup;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit- notebook isn't two words, therefore noteBook -> notebook


export class OneNoteItemUtils {
/**
* Given the id of the OneNoteItem, and a notebook or sectionGroup list, returns
Expand Down Expand Up @@ -98,6 +100,38 @@ export class OneNoteItemUtils {

return ancestry;
}

/**
* Finds the maximum depth of notebooks list, including sections
*/
getDepthOfNotebooks(notebooks: Notebook[]): number {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this being used?

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be good to unit test this method

if (!notebooks || notebooks.length === 0) {
return 0;
}
return notebooks.map((notebook) => this.getDepthOfParent(notebook)).reduce((d1, d2) => Math.max(d1, d2));
}

/**
* Finds the maximum depth of SectionGroup or Notebook,
* includes the number of sections below it
*/
getDepthOfParent(parent: noteBookOrSectionGroup ): number {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be private?

if (!parent) {
return 0;
}

let containsAtLeastOneSection = parent.sections && parent.sections.length > 0;
let maxDepth = containsAtLeastOneSection ? 1 : 0;

if (parent.sectionGroups) {
for (let i = 0; i < parent.sections.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant parent.sectiongroups here. Also, I suggest using
for(const sectionGroup of parent.sectionGroups){
...
instead of a for with indexes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the sectionGroups thing is actually to measure the sections here- it looks weird but changing these around breaks it. I'm mostly copying this code over from the utils in the OneNoteAPI repo we were using before

Copy link
Contributor

@jogonzal jogonzal Apr 30, 2018

Choose a reason for hiding this comment

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

The code says:


if (parent.sectionGroups) {
for (let i = 0; i < parent.sections.length; i++) {
maxDepth = Math.max(this.getDepthOfParent(parent.sectionGroups[i]), maxDepth);
}
}

Let's suppose this parent has 1 section groups and 10 sections. It goes inside the if, loops from 0 to 9, and tries to access parent.sectionGroups[i]; It will throw when i=1 because the parent only has 1 section group.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean there's a bug in the OneNoteApi repo we should fix?

maxDepth = Math.max(this.getDepthOfParent(parent.sectionGroups[i]), maxDepth);
}
}

// Include the parent itself
return maxDepth + 1;
}
}

export * from './oneNoteApiResponseTransformer';