') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Remove order parameter admin view by marcellamaki · Pull Request #2653 · learningequality/studio · GitHub
Skip to content

Remove order parameter admin view - #2653

Merged
rtibbles merged 3 commits into
learningequality:developfrom
marcellamaki:remove-order-parameter-admin-view
Dec 9, 2020
Merged

Remove order parameter admin view#2653
rtibbles merged 3 commits into
learningequality:developfrom
marcellamaki:remove-order-parameter-admin-view

Conversation

@marcellamaki

@marcellamakimarcellamaki commented Dec 9, 2020

Copy link
Copy Markdown
Member

Description

This PR removes the 'ordering' param from being set in the front end, and the backend code that filters this param, because it is not used for the tables in /administration. It duplicates information that already exists in sortBy and descending.

Issue Addressed (if applicable)

Addresses issues discussed in PR 2617

Before/After Screenshots (if applicable)

Before - information duplicated:
Screen Shot 2020-12-08 at 6 15 18 PM

After - ordering not passed in URL params:
no-ordering

Steps to Test

  • Navigate to the /administration page
  • Test the sortBy feature by toggling ascending and descending on various table columns. This should work with no issues on both Channels and Users

Comments

I did not see any issues with removing this and I find both the front end and the back end seem to be working as expected with manual testing. It is possible that there are issues with this approach that I did not foresee, so any feedback, especially regarding the backend, would be appreciate.

@codecov

codecovBot commented Dec 9, 2020•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #2653 (1d2cf70) into develop (47605c5) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@## develop #2653 +/- ##
===========================================
- Coverage 85.10% 85.09% -0.01% 
===========================================
Files 295 295 Lines 15018 15011 -7 ===========================================
- Hits 12781 12774 -7 
Misses 2237 2237
Impacted FilesCoverage Δ
...ontentcuration/contentcuration/viewsets/channel.py78.37% <ø> (-0.22%)⬇️
contentcuration/contentcuration/viewsets/user.py71.92% <ø> (-0.41%)⬇️
...ontentcuration/contentcuration/utils/pagination.py84.09% <100.00%> (-0.36%)⬇️

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 47605c5...1d2cf70. Read the comment docs.

jredrejo
jredrejo approved these changes Dec 9, 2020
View reviewed changes

@jredrejojredrejo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @marcellamaki , this is great,
would you mind removing the code added at https://github.com/learningequality/studio/blob/develop/contentcuration/contentcuration/utils/pagination.py#L67 to support this unnedeed ordering param?
If you have doubts, no worries, I can do it afterwards

@marcellamaki

marcellamaki commented Dec 9, 2020

Copy link
Copy Markdown
MemberAuthor

Hi @jredrejo I have removed the code in that function and I think it is working fine, but if you see any problems, please just let me know and I can adjust. Thank you!

@jredrejo

jredrejo commented Dec 9, 2020

Copy link
Copy Markdown
Contributor

Hi @jredrejo I have removed the code in that function and I think it is working fine, but if you see any problems, please just let me know and I can adjust. Thank you!

Your code will work without problems, I would have done it differently, but the logic is fine, thanks!

@rtibbles
rtibbles merged commit 69efd21 into learningequality:developDec 9, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonboiserjonboiserAwaiting requested review from jonboiser
1 more reviewer
@jredrejojredrejojredrejo approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@marcellamaki@jredrejo@rtibbles