Skip to content

Don't filter for ancestors if target is root node - #2992

Merged
rtibbles merged 2 commits into
learningequality:hotfixesfrom
bjester:root-ancestors-call
Feb 25, 2021
Merged

Don't filter for ancestors if target is root node#2992
rtibbles merged 2 commits into
learningequality:hotfixesfrom
bjester:root-ancestors-call

Conversation

@bjester

@bjesterbjester commented Feb 25, 2021

Copy link
Copy Markdown
Member

Description

This issue was caused by a pending call to get the ancestors of the root node of the channel, which blocked the loading of the page and thereby prevented copying. Unfortunately, this ancestors call can take several seconds on hotfixes and production (90 seconds on hotfixes for me once), even though the root node has no ancestors. This PR makes a change to take a shortcut in that scenario to only return the target node.

Issue Addressed (if applicable)

Addresses #2927
Addresses #2889

Steps to Reproduce/Test

  • Create a new channel on hotfixes
  • Open the channel
  • While the ancestors call is pending, you should see an empty gray channel
  • Copying is not possible until the ancestors call completes and you see "Click ADD..."
  • Once the API call completes, which should be much faster with this change, you should see the screenshot below and should be able to drag and drop from clipboard to the channel
    Screenshot from 2021-02-24 15-30-59

Checklist

  • Is the code clean and well-commented?
  • Are there tests for this change?

@codecov

codecovBot commented Feb 25, 2021

Copy link
Copy Markdown

Codecov Report

Merging #2992 (5c2410f) into hotfixes (28587f8) will increase coverage by 4.99%.
The diff coverage is 92.92%.

Impacted file tree graph

@@ Coverage Diff @@## hotfixes #2992 +/- ##
============================================
+ Coverage 80.80% 85.80% +4.99% 
============================================
Files 281 298 +17 Lines 12659 15875 +3216 ============================================
+ Hits 10229 13621 +3392 + Misses 2430 2254 -176 
Impacted FilesCoverage Δ
contentcuration/contentcuration/decorators.py56.60% <50.00%> (-37.60%)⬇️
...ntentcuration/contentcuration/db/models/manager.py91.20% <90.98%> (-8.80%)⬇️
contentcuration/contentcuration/forms.py82.35% <94.04%> (+33.89%)⬆️
contentcuration/contentcuration/api.py92.06% <100.00%> (+1.43%)⬆️
...tentcuration/contentcuration/context_processors.py100.00% <100.00%> (ø)
...ontentcuration/contentcuration/db/advisory_lock.py100.00% <100.00%> (ø)
...tcuration/contentcuration/db/models/expressions.py93.33% <100.00%> (-6.67%)⬇️
...entcuration/contentcuration/db/models/functions.py100.00% <100.00%> (ø)
...ation/contentcuration/tests/test_rest_framework.py36.93% <0.00%> (-63.07%)⬇️
contentcuration/contentcuration/utils/format.py21.05% <0.00%> (-49.54%)⬇️
... and 181 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3364062...7038f92. Read the comment docs.

@rtibbles
rtibbles merged commit 67f2ad3 into learningequality:hotfixesFeb 25, 2021
@bjester
bjester deleted the root-ancestors-call branch April 27, 2021 20:30
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants

@bjester@rtibbles