Fix front-end Helpers.ifHasPermissions #1345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frontend/src/core/app/helpers.js
Outdated
| ifUserIsMe: function(userId, block) { | ||
| if (userId === Origin.sessionModel.get('id')) { | ||
| return block.fn(this); | ||
| } else { | ||
| return block.inverse(this); | ||
| } | ||
| }, | ||
|
|
||
| selected: function(option, value){ | ||
| if (option === value) { | ||
| return ' selected'; | ||
| } else { | ||
| return '' |
There was a problem hiding this comment.
Missing semi-colon.
frontend/src/core/app/helpers.js
Outdated
| @@ -221,8 +237,7 @@ define(function(require){ | |||
| }, | |||
There was a problem hiding this comment.
Line 232, update to strict equality:
url.indexOf('course/assets') === 0
frontend/src/core/app/helpers.js
Outdated
| @@ -253,47 +268,45 @@ define(function(require){ | |||
| }, | |||
There was a problem hiding this comment.
Line 262 missing semi-colon.
frontend/src/core/app/helpers.js
Outdated
| counterFromOne: function(n, block) { | ||
| var sum = ''; | ||
| for (var i = 1; i <= n; ++i) | ||
| sum += block.fn(i); | ||
| return sum; | ||
| }, | ||
|
|
||
| t: function(str, options) { | ||
| for (var placeholder in options.hash) { | ||
| options[placeholder] = options.hash[placeholder]; | ||
| } | ||
| return (window.polyglot != undefined ? window.polyglot.t(str, options) : str); |
There was a problem hiding this comment.
Update to strict equality:
window.polyglot !== undefined
frontend/src/core/app/helpers.js
Outdated
| stripHtml: function(html) { | ||
| return new Handlebars.SafeString(html); | ||
| }, | ||
|
|
||
| bytesToSize: function(bytes) { | ||
| if (bytes == 0) return '0 B'; |
There was a problem hiding this comment.
Update to strict equality:
bytes === 0
|
Hi, any update on what's happening with this PR? I've just encountered this bug but see there's already a fix for it... PR looks a bit stale though. |
|
@canstudios-davidl I'll pick this up for the next release |
|
Thanks @taylortom happy to help if needed |
|
Happy to approve once the conflicts are resolved. |
* develop: (409 commits) ignore .vscode and package-lock.json supress npm warnings article view now generated after article model saved Fixes #1772 Amend check to allow undefined data Switch to local log func Stop throwing error if repo update check failed Fix version check Add override for removal of list items Handle response errors better amend title for clarity following code review better help text, more consistent use of '?' in titles remove some ARIA labels that were not used add the core properties _isVisible and _isHidden add missing 'instruction' property to article & block improve usability through better titles/help text Master changes (#1741) Fix bug with recursive function call Remove testing header Update CHANGELOG for release 0.4.0 (#1739) Enchancements to install/update (#1726) Amend contentModel._children to allow for arrays fixes 1474 ... # Conflicts: # frontend/src/core/helpers.js
canstudios-nicolaw
previously approved these changes
Mar 2, 2018
* develop: (80 commits) update Check if user is course owner Require modules before plugins Change to ensureDirSync refactor to set scrollTop after all animations are completed hook into transition end event instead of post render Fixes #1906 Allow MongoDB authentication in install script Fix modules define Fix globalData Make sure we load the plugin Fix issue with merge Save isVisible and isHidden on blocks and components Fix context menu for page Move plugin folder to modules Refactor for readability Update comments Fix plugin code broken in refactor Persist sort prefs indefinitely Remove config-sample.json ... # Conflicts: # frontend/src/core/helpers.js
canstudios-louisem
approved these changes
Mar 21, 2018
canstudios-nicolaw
approved these changes
Mar 27, 2018
dancgray
approved these changes
Apr 4, 2018
| pickCSV: function (list, key, separator) { | ||
| var vals = []; | ||
| separator = (separator && separator.length) ? separator : ','; | ||
| if (list && list.length) { |
There was a problem hiding this comment.
Looks like the logic hasn't been reversed here when returning early.
Anyhow, suggest refactoring to return right at the start of the function. Something like:
pickCSV: function(list, key, separator) {
if (!list || !list.length) {
return '';
}
var vals = [];
if (!separator || !separator.length) {
separator = ',';
}
for (var i = 0, l = list.length; i < l; i++) {
var val = list[i];
var nestedVal = key && val[key];
vals.push(nestedVal || val);
}
return vals.join(separator);
}| }, | ||
|
|
||
| ifImageIsCourseAsset: function(url, block) { | ||
| var isCourseAsset = url.length !== 0 && url.indexOf('course/assets') == 0; |
There was a problem hiding this comment.
Please use triple equals: url.indexOf('course/assets') === 0
| } | ||
| }, | ||
| bytesToSize: function(bytes) { | ||
| if (bytes == 0) return '0 B'; |
There was a problem hiding this comment.
Please use triple equals: bytes === 0.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1344