Add complete field to edit modal - #2281

Merged
rtibbles merged 8 commits into
learningequality:developfrom
MisRob:validation-complete
Sep 24, 2020
Merged

Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom
MisRob:validation-complete

Conversation

@MisRob

@MisRobMisRob commented Sep 22, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Description

The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.

The second commit implements logic related to setting a node as complete.

All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.

Steps to Test

  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.

Notes

Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283

Checklist

  • 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)?

Michaela Robosova added 5 commits September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob added this to the Vue Refactor milestone Sep 22, 2020
@codecov

codecovBot commented Sep 22, 2020•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #2281 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)

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 d730695...9086d99. Read the comment docs.

jayoshih
jayoshih previously requested changes Sep 22, 2020
View reviewed changes

@jayoshihjayoshih 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.

Nice work! Just a few comments

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js Outdated
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob

MisRob commented Sep 23, 2020

Copy link
Copy Markdown
MemberAuthor

Thank you @jayoshih, the PR has just been updated.

jayoshih
jayoshih reviewed Sep 23, 2020
View reviewed changes

@jayoshihjayoshih 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.

Just one check...

Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue Outdated
@rtibbles
rtibbles merged commit 11176e6 into learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis mentioned this pull request Sep 30, 2020
Merged
@MisRob
MisRob deleted the validation-complete branch November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Add complete field to edit modal - #2281
Merged
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Sep 24, 2020
Merged
Add complete field to edit modal#2281
rtibbles merged 8 commits into
learningequality:developfrom 
MisRob:validation-complete
Conversation
@MisRob
@MisRobMisRob
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Member
Description
The first commit moves all validation rules to one source to make sure that we won't run into inconsistencies now when frontend is responsible for setting a node as complete.
The second commit implements logic related to setting a node as complete.
All remaining commits are related mainly to cleanup related to that we also use validation rules in some shared components, which means that they should be moved to shared directory instead of staying in the channel edit app.
Steps to Test
  • Go to edit modal
  • Create/update nodes and check that on /sync, a correct complete value is sent in mods (please notice that it will be sent only when its new value is different from the previous one - this is how our sync works in general). There are complete/incomplete criteria you can use for testing.
Notes
Flagging that while wrapping this up, I've noticed two validation issues on develop that might be related to some recent changes. I've just reported both of them:
#2282
#2283
Checklist
  •  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)?
Michaela Robosova
added 5 commits
September 22, 2020 16:27
Michaela Robosova
One source for validation rules
b7b0968
Michaela Robosova
Add complete logic
ed88aec
Michaela Robosova
Remove updateContentNodes action
2124191
that's not called from anywhere at this point.
If we need such action in the future, we will need
to take into account `complete` validation logic
instead of using the same payload for all content
nodes.
Michaela Robosova
Move shared utils to their own directory
07deaae
Michaela Robosova
Move shared validation logic to shared directory
b7fddab
@MisRobMisRob
added this to the Vue Refactor milestone
Sep 22, 2020
@codecov
codecovBot
commented
Sep 22, 2020•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #2281 into develop will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## develop #2281 +/- ##
========================================
Coverage 81.48% 81.48% ========================================
Files 293 293 Lines 14066 14066 ========================================
Hits 11462 11462 Misses 2604 2604 
Impacted FilesCoverage Δ
contentcuration/contentcuration/views/base.py55.31% <0.00%> (ø)
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 d730695...9086d99. Read the comment docs.
jayoshih
jayoshih
previously requested changes
Sep 22, 2020
View reviewed changes
@jayoshihjayoshih
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.

Nice work! Just a few comments
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Outdated

Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
Comment threadcontentcuration/contentcuration/frontend/shared/utils/validation.js
@MisRob
MisRob
commented
Sep 23, 2020

Copy link


Copy Markdown

MemberAuthor
Thank you @jayoshih, the PR has just been updated.
jayoshih
jayoshih
reviewed
Sep 23, 2020
View reviewed changes
@jayoshihjayoshih
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.

Just one check...
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated

@rtibbles
rtibbles
merged commit 11176e6
into
learningequality:developSep 24, 2020
@nucleogenesisnucleogenesis
mentioned this pull request
Sep 30, 2020
 Merged

@MisRob
MisRob
deleted the

validation-complete

branch
November 3, 2020 06:18
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes
@jayoshihjayoshihAwaiting requested review from jayoshih

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone

Vue Refactor

Development

Successfully merging this pull request may close these issues.

3 participants

@MisRob@rtibbles@jayoshih