Add guard to TreeViewcreated hook, to prevent data over-fetching

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.

[jonboiser]

Description

This adds a guard to the TreeView's created hook to prevent it (along with the NodePanels created hook) from doing two types of overfetching:

1. At a channel's root node, TreeView and NodePanel will collectively request the root node twice
2. At a channels' non-root node Treeview and NodePanel will collectively request the root node once, and the current node twice

Here I'm using a shorthand of "request the node" to mean "make a call to /api/contentnode with the query param of parent=node".

After this fix:

1. At a channel's root node, TreeView will not request the root node. And NodePanel will request the root node once.
2. At a channels' non-root node Treeview will request the root node once. And NodePanel will request the current node once.

Issue Addressed (if applicable)

Fixes#2316

Before/After Screenshots (if applicable)

Before:

Notice that the same IDs are repeated multiple times in the request URL

After:

Notice that the ID do not overlap between requests URLs

Steps to Test

• Step 1
• Step 2

Implementation Notes (optional)

At a high level, how did you implement this?

I added a guard to TreeView to ask for a smaller amount of data, with the knowledge that NodePanel will be rendered in the UI tree and make a complementary data request. This ensures that the current behavior of always request the root node + the current node is retained, but done in a way that we are not overfetching duplicate copies of these nodes.

Does this introduce any tech-debt items?

List anything that will need to be addressed later

I don't know enough about the architecture to know if this is hacky. It's possible that a smarter solution would have just removed all calls to loadContentNodes from TreeView and deferred it to NodePanel

Checklist

Delete any items that don't apply

• Is the code clean and well-commented?
• Has the docs label been added if this introduces a change that needs to be updated in the user docs?
• Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
• Are there tests for this change?
• Are all user-facing strings translated properly (if applicable)?
• Has the notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)?
• Are all UI components LTR and RTL compliant (if applicable)?
• Are views organized into pages, components, and layouts directories as described in the docs?
• Are there any new ways this uses user data that needs to be factored into our Privacy Policy?
• Are there any new interactions that need to be added to the QA Sheet?
• Are there opportunities for using Google Analytics here (if applicable)?
• If the Pipfile has been changed, is the updated Pipfile.lock file also included in this PR?
• Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

• Jordan jayoshih (full stack)
• Aron aronasorman (back end, devops)
• Micah micahscopes (full stack)
• Kevin kollivier (back end)
• Ivan ivanistheone (Ricecooker)
• Richard rtibbles (full stack, Kolibri)
• Radina @radinamatic (documentation)

[micahscopes]

This seems good for now, and I tried it out and it works. It also doesn't seem like it'd be too difficult to rework this later if someone felt inspired to.