Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix/TR-3995/Max words restriction can be exceeded #145

Merged
merged 7 commits into from
Jun 17, 2022
Merged

Fix/TR-3995/Max words restriction can be exceeded #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
61b86d1
test(strLimiter): rewrite the unit tests using parametric input
jsconan May 3, 2022
34dbb99
test(strLimiter): add more test cases for the limit by words
jsconan May 4, 2022
79a0023
fix(strLimiter): rewrite the limiter by words, taking HTML content in…
jsconan May 4, 2022
da34f1b
test(strLimiter): add test cases for empty and negative limit
jsconan May 4, 2022
e8268b4
fix(strLimiter): make sure a negative limit is considered as 0
jsconan May 4, 2022
2bab06f
test(strLimiter): prepare test cases for the limit by chars
jsconan May 5, 2022
4057299
fix(strLimiter): rewrite the limiter by chars, taking HTML content in…
jsconan May 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 90 additions & 24 deletions src/util/strLimiter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,50 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
* Copyright (c) 2018-2019 Open Assessment Technologies SA
*
* Copyright (c) 2018-2022 Open Assessment Technologies SA
*/

/**
* Limit a string by the supplied limiter function.
* @param {string} text
* @param {function} limitText
* @returns {string}
*/
function limitBy(text, limitText) {
/**
* Limits the size of an HTML fragment, removing the extraneous content.
* @param {Node} fragment
*/
const limitFragment = fragment => {
[].slice.call(fragment.childNodes).forEach(node => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: may be we can rely on ES6 equivalent Array.from() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I was considering using it. But as there might be a need to backport it to an older version than needs IE support I preferred to stay safe.

switch (node.nodeType) {
case Node.ELEMENT_NODE:
if (node.childNodes.length && node.textContent.trim()) {
limitFragment(node);

if (!node.textContent.trim()) {
node.remove();
}
}
break;

case Node.TEXT_NODE:
node.textContent = limitText(node.textContent);
break;
}
});
};

if (/<.*>/g.test(text)) {
const fragment = document.createElement('div');
fragment.innerHTML = text;
limitFragment(fragment);
return fragment.innerHTML;
}

return limitText(text);
}

/**
* Limit a string by either word or character count
*
Expand All @@ -25,36 +65,62 @@

export default {
/**
* Limit a string by word count
* Limits a string by word count.
*
* @param {string} str
* @param {integer} maxWordCount
* @param {string} text
* @param {number} limit
* @returns {string}
*/
limitByWordCount: function limitByWordCount(str, maxWordCount) {
// contains alternating a word and whitespace
// to make sure the original whitespace is retained
var textArr = str.match(/(([\S]+)|([\s]+))/g);
var newText = /\s+/.test(textArr[0]) ? textArr.shift() : '';
while (maxWordCount && textArr.length) {
newText += textArr.shift(); // word
if (textArr.length) {
newText += textArr.shift(); // white space
limitByWordCount(text, limit) {
/**
* Cuts a plain text after the max number of words expressed by the variable `limit`.
* @param {string} str
* @returns {string}
*/
const limitText = str => {
// split words by space, keeping the leading spaces attached
const words = str.match(/([\s]*[\S]+)/g);
// keep the trailing spaces
const trailing = str.match(/(\s+)$/);

if (!words) {
return '';
}
maxWordCount--;
}
newText = newText.replace(/\s+$/, ''); // remove trailing space
return newText;

const count = Math.max(0, limit);
limit = Math.max(0, count - words.length);
return words.slice(0, count).join('') + ((trailing && trailing[0]) || '');
};

return limitBy(text, limitText).replace(/(\s+)$/, '');
},

/**
* Limit a string by character count
* Limit a string by character count.
*
* @param {string} str
* @param {integer} maxCharCount
* @returns {string|*}
* @param {string} text
* @param {number} limit
* @returns {string}
*/
limitByCharCount: function limitByCharCount(str, maxCharCount) {
return str.substr(0, maxCharCount);
limitByCharCount(text, limit) {
/**
* Cuts a plain text after the max number of chars expressed by the variable `limit`.
* @param {string} str
* @returns {string}
*/
const limitText = str => {
// split by char or by HTML entity
const chars = str.match(/((&.*?;)|(.))/g);

if (!chars) {
return '';
}

const count = Math.max(0, limit);
limit = Math.max(0, count - chars.length);
return chars.slice(0, count).join('');
};

return limitBy(text, limitText);
}
};
148 changes: 103 additions & 45 deletions test/util/strLimiter/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,54 +13,112 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
* Copyright (c) 2018 Open Assessment Technologies SA
*
*
* Copyright (c) 2018-2022 Open Assessment Technologies SA
*/
define(['util/strLimiter'], function(strLimiter) {
define(['util/strLimiter'], function (strLimiter) {
'use strict';

var txt = 'Lorem ipsum dolor sit amet'; // 5 words, 26 characters

var weirdWhiteSpace = 'Lorem ipsum dolor sit amet';

QUnit.module('API');

QUnit.test('Limit by Word Count', function(assert) {
assert.equal(
strLimiter.limitByWordCount(txt, 5),
'Lorem ipsum dolor sit amet',
'Word count, input already correct size'
);
assert.equal(strLimiter.limitByWordCount(txt, 10), 'Lorem ipsum dolor sit amet', 'Word count, input too short');
assert.equal(strLimiter.limitByWordCount(txt, 2), 'Lorem ipsum', 'Word count, input too long');
});
QUnit.module('strLimiter');

QUnit.test('Limit by Word Count, weird whitespace', function(assert) {
assert.equal(
strLimiter.limitByWordCount(weirdWhiteSpace, 5),
'Lorem ipsum dolor sit amet',
'Word count, input already correct size'
);
assert.equal(
strLimiter.limitByWordCount(weirdWhiteSpace, 10),
'Lorem ipsum dolor sit amet',
'Word count, input too short'
);
assert.equal(strLimiter.limitByWordCount(weirdWhiteSpace, 2), 'Lorem ipsum', 'Word count, input too long');
});
QUnit.cases
.init([
{
title: 'plain text',
input: 'Lorem ipsum dolor sit amet',
unlimited: 'Lorem ipsum dolor sit amet',
limited: 'Lorem ipsum'
},
{
title: 'plain text with long spaces',
input: ' Lorem ipsum dolor sit amet ',
unlimited: ' Lorem ipsum dolor sit amet',
limited: ' Lorem ipsum'
},
{
title: 'simple HTML',
input: '<p>Lorem ipsum dolor sit amet</p>',
unlimited: '<p>Lorem ipsum dolor sit amet</p>',
limited: '<p>Lorem ipsum</p>'
},
{
title: 'glued HTML',
input: '<p> Lorem </p><p> ipsum <br></p><p> dolor </p><p> sit </p><p> amet </p>',
unlimited: '<p> Lorem </p><p> ipsum <br></p><p> dolor </p><p> sit </p><p> amet </p>',
limited: '<p> Lorem </p><p> ipsum <br></p>'
}
])
.test('limitByWordCount ', (data, assert) => {
assert.equal(strLimiter.limitByWordCount(data.input, 5), data.unlimited, 'Limited by 5 words');
assert.equal(strLimiter.limitByWordCount(data.input, 10), data.unlimited, 'Limited by 10 words');
assert.equal(strLimiter.limitByWordCount(data.input, 2), data.limited, 'Limited by 2 words');
assert.equal(strLimiter.limitByWordCount(data.input, 0), '', 'Limited by 0 words');
assert.equal(strLimiter.limitByWordCount(data.input, -2), '', 'Limited by negative value');
});

QUnit.test('Limit by Character count', function(assert) {
assert.equal(
strLimiter.limitByCharCount(txt, 26),
'Lorem ipsum dolor sit amet',
'Char count, input already correct size'
);
assert.equal(
strLimiter.limitByCharCount(txt, 100),
'Lorem ipsum dolor sit amet',
'Char count, input too short'
);
assert.equal(strLimiter.limitByCharCount(txt, 11), 'Lorem ipsum', 'Char count, input too long');
});
QUnit.cases
.init([
{
title: 'plain text',
length: 26,
limit: 11,
input: 'Lorem ipsum dolor sit amet',
unlimited: 'Lorem ipsum dolor sit amet',
limited: 'Lorem ipsum'
},
{
title: 'plain text with long spaces',
length: 38,
limit: 19,
input: ' Lorem ipsum dolor sit amet ',
unlimited: ' Lorem ipsum dolor sit amet ',
limited: ' Lorem ipsum '
},
{
title: 'plain text with entities',
length: 36,
limit: 14,
input: 'Lorem &nbsp;&nbsp; ipsum &nbsp; dolor &eacute; sit &agrave; amet',
unlimited: 'Lorem &nbsp;&nbsp; ipsum &nbsp; dolor &eacute; sit &agrave; amet',
limited: 'Lorem &nbsp;&nbsp; ipsum'
},
{
title: 'simple HTML',
length: 26,
limit: 11,
input: '<p>Lorem ipsum dolor sit amet</p>',
unlimited: '<p>Lorem ipsum dolor sit amet</p>',
limited: '<p>Lorem ipsum</p>'
},
{
title: 'glued HTML',
length: 32,
limit: 14,
input: '<p> Lorem </p><p> ipsum <br></p><p> dolor </p><p> sit </p><p> amet </p>',
unlimited: '<p> Lorem </p><p> ipsum <br></p><p> dolor </p><p> sit </p><p> amet </p>',
limited: '<p> Lorem </p><p> ipsum <br></p>'
},
{
title: 'simple HTML with entities',
length: 36,
limit: 14,
input: '<p>Lorem &nbsp;&nbsp; ipsum &nbsp; dolor &nbsp; sit &nbsp; amet</p>',
unlimited: '<p>Lorem &nbsp;&nbsp; ipsum &nbsp; dolor &nbsp; sit &nbsp; amet</p>',
limited: '<p>Lorem &nbsp;&nbsp; ipsum</p>'
}
])
.test('limitByCharCount ', (data, assert) => {
assert.equal(
strLimiter.limitByCharCount(data.input, data.length),
data.unlimited,
`Limit by ${data.length} characters`
);
assert.equal(strLimiter.limitByCharCount(data.input, 100), data.unlimited, `Limit by 100 characters`);
assert.equal(
strLimiter.limitByCharCount(data.input, data.limit),
data.limited,
`Limit by ${data.limit} characters`
);
assert.equal(strLimiter.limitByCharCount(data.input, 0), '', 'Limit by 0 characters');
assert.equal(strLimiter.limitByCharCount(data.input, -2), '', 'Limit by negative value');
});
});