From 69000ee25ce48c769bd8dcc6dbd14fa91f97b3d9 Mon Sep 17 00:00:00 2001
From: Emmanuel Demey <demey.emmanuel@gmail.com>
Date: Fri, 5 Jul 2019 16:53:59 +0200
Subject: [PATCH 1/3] fix(pagination): Disable buttons if they cannot be used

---
 src/js/components/shared/pagination/index.js  | 54 ++++++++++---------
 .../shared/pagination/pagination.scss         |  4 ++
 .../shared/pagination/pagination.spec.js      | 16 ++++++
 3 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/src/js/components/shared/pagination/index.js b/src/js/components/shared/pagination/index.js
index 3700521dd..dad0cf49d 100644
--- a/src/js/components/shared/pagination/index.js
+++ b/src/js/components/shared/pagination/index.js
@@ -12,8 +12,20 @@ function checkInvalidPage(targetPage, listSize) {
  *	context: The context of the page. Used for theming
  */
 class Pagination extends Component {
-	constructor() {
-		super();
+	static propTypes = {
+		itemEls: PropTypes.arrayOf(PropTypes.element).isRequired,
+		itemsPerPage: PropTypes.string.isRequired,
+		context: PropTypes.oneOf([
+			'',
+			'concepts',
+			'collections',
+			'classifications',
+			'operations',
+		]),
+	};
+
+	constructor(props) {
+		super(props);
 		this.state = {
 			currentPage: 1,
 		};
@@ -46,20 +58,18 @@ class Pagination extends Component {
 			pageNumbers.push(i);
 		}
 
-		function activePage(page) {
-			return page === currentPage ? 'page-item active' : 'page-item';
+		function isActivePage(page) {
+			return page === currentPage;
 		}
 		function isDisabled(targetPage) {
-			return checkInvalidPage(targetPage, pageNumbers.length)
-				? 'page-item disabled'
-				: 'page-item';
+			return checkInvalidPage(targetPage, pageNumbers.length);
 		}
 
 		const renderPageNumbers = pageNumbers
 			.filter(number => number - 3 < currentPage && number + 3 > currentPage)
 			.map(number => {
 				return (
-					<li className={activePage(number)} key={number} id={number}>
+					<li className={isActivePage(number) && 'active'} key={number}>
 						<button
 							href="#"
 							onClick={e => this.goToPage(number, e)}
@@ -77,13 +87,17 @@ class Pagination extends Component {
 				<ul className="list-group">{currentItems}</ul>
 				{pageNumbers.length > 1 && (
 					<ul className={`pagination pg-rmes ${contextCSS}`}>
-						<li className="page-item">
-							<button onClick={e => this.goToPage(1, e)} aria-label="First">
+						<li className={isDisabled(currentPage - 1) && 'disabled'}>
+							<button
+								onClick={e => this.goToPage(1, e)}
+								aria-label="First"
+								disabled={isDisabled(currentPage - 1)}
+							>
 								<span aria-hidden="true">&laquo;</span>
 								<span className="sr-only">First</span>
 							</button>
 						</li>
-						<li className={isDisabled(currentPage - 1)}>
+						<li className={isDisabled(currentPage - 1) && 'disabled'}>
 							<button
 								href="#"
 								onClick={e =>
@@ -91,13 +105,14 @@ class Pagination extends Component {
 									this.goToPage(currentPage - 1, e)
 								}
 								aria-label="Previous"
+								disabled={isDisabled(currentPage - 1)}
 							>
 								<span aria-hidden="true">&lt;</span>
 								<span className="sr-only">Previous</span>
 							</button>
 						</li>
 						{renderPageNumbers}
-						<li className={isDisabled(currentPage + 1)}>
+						<li className={isDisabled(currentPage + 1) && 'disabled'}>
 							<button
 								href="#"
 								onClick={e =>
@@ -105,18 +120,20 @@ class Pagination extends Component {
 									this.goToPage(currentPage + 1, e)
 								}
 								aria-label="Next"
+								disabled={isDisabled(currentPage + 1)}
 							>
 								<span aria-hidden="true">&gt;</span>
 								<span className="sr-only">Next</span>
 							</button>
 						</li>
-						<li className="page-item">
+						<li className={isDisabled(currentPage + 1) && 'disabled'}>
 							<button
 								aria-label="Last"
 								href="#"
 								onClick={e =>
 									this.goToPage(pageNumbers[pageNumbers.length - 1], e)
 								}
+								disabled={isDisabled(currentPage + 1)}
 							>
 								<span aria-hidden="true">&raquo;</span>
 								<span className="sr-only">Last</span>
@@ -129,15 +146,4 @@ class Pagination extends Component {
 	}
 }
 
-Pagination.propTypes = {
-	itemEls: PropTypes.arrayOf(PropTypes.element).isRequired,
-	itemsPerPage: PropTypes.string.isRequired,
-	context: PropTypes.oneOf([
-		'',
-		'concepts',
-		'collections',
-		'classifications',
-		'operations',
-	]),
-};
 export default Pagination;
diff --git a/src/js/components/shared/pagination/pagination.scss b/src/js/components/shared/pagination/pagination.scss
index cea46fbff..21f494dbe 100644
--- a/src/js/components/shared/pagination/pagination.scss
+++ b/src/js/components/shared/pagination/pagination.scss
@@ -11,6 +11,10 @@
 		text-decoration: none;
 		border: 1px solid #ddd;
 
+		&.disabled {
+			color: lightgrey;
+		}
+
 		&:first-child {
 			border-top-left-radius: 5px;
 			border-bottom-left-radius: 5px;
diff --git a/src/js/components/shared/pagination/pagination.spec.js b/src/js/components/shared/pagination/pagination.spec.js
index dee1d4c32..5f647b1a1 100644
--- a/src/js/components/shared/pagination/pagination.spec.js
+++ b/src/js/components/shared/pagination/pagination.spec.js
@@ -127,6 +127,7 @@ describe('pagination', () => {
 		);
 		wrapper.setState({ currentPage: 2 });
 		expect(wrapper.find('.disabled').length).toBe(0);
+		expect(wrapper.find('button[disabled=true]').length).toBe(0);
 	});
 	it('should disable the previous link if we are on the page 1', () => {
 		const wrapper = shallow(
@@ -140,6 +141,14 @@ describe('pagination', () => {
 				.at(1)
 				.hasClass('disabled')
 		).toBeTruthy();
+
+		expect(
+			wrapper
+				.find('.pagination')
+				.find('li')
+				.at(0)
+				.hasClass('disabled')
+		).toBeTruthy();
 	});
 
 	it('should disable the next link if we are on the last page', () => {
@@ -155,5 +164,12 @@ describe('pagination', () => {
 				.at(5)
 				.hasClass('disabled')
 		).toBeTruthy();
+		expect(
+			wrapper
+				.find('.pagination')
+				.find('li')
+				.at(6)
+				.hasClass('disabled')
+		).toBeTruthy();
 	});
 });

From 1a790cdde8fac92b5835643fa077c488174659c4 Mon Sep 17 00:00:00 2001
From: Emmanuel Demey <demey.emmanuel@gmail.com>
Date: Fri, 5 Jul 2019 17:04:36 +0200
Subject: [PATCH 2/3] fix(pagination): Solve unit test

---
 .../components/shared/pagination/pagination.spec.js  | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/src/js/components/shared/pagination/pagination.spec.js b/src/js/components/shared/pagination/pagination.spec.js
index 5f647b1a1..8056fa21a 100644
--- a/src/js/components/shared/pagination/pagination.spec.js
+++ b/src/js/components/shared/pagination/pagination.spec.js
@@ -61,8 +61,8 @@ describe('pagination', () => {
 		);
 		expect(wrapper.state(['currentPage'])).toBe(1);
 		wrapper
-			.find('.pagination')
-			.find('[id=2] button')
+			.findWhere(node => node.key() === '2')
+			.find('button')
 			.first()
 			.simulate('click', e);
 		expect(wrapper.state(['currentPage'])).toBe(2);
@@ -75,7 +75,6 @@ describe('pagination', () => {
 		wrapper.setState({ currentPage: 2 });
 
 		wrapper
-			.find('.pagination')
 			.find('button')
 			.at(1)
 			.simulate('click', e);
@@ -89,7 +88,6 @@ describe('pagination', () => {
 		wrapper.setState({ currentPage: 2 });
 
 		wrapper
-			.find('.pagination')
 			.find('button')
 			.at(6)
 			.simulate('click', e);
@@ -102,7 +100,6 @@ describe('pagination', () => {
 		);
 		expect(wrapper.state(['currentPage'])).toBe(1);
 		wrapper
-			.find('.pagination')
 			.find('button')
 			.first()
 			.simulate('click', e);
@@ -114,7 +111,6 @@ describe('pagination', () => {
 		);
 		expect(wrapper.state(['currentPage'])).toBe(1);
 		wrapper
-			.find('.pagination')
 			.find('button')
 			.last()
 			.simulate('click', e);
@@ -136,7 +132,6 @@ describe('pagination', () => {
 		wrapper.setState({ currentPage: 1 });
 		expect(
 			wrapper
-				.find('.pagination')
 				.find('li')
 				.at(1)
 				.hasClass('disabled')
@@ -144,7 +139,6 @@ describe('pagination', () => {
 
 		expect(
 			wrapper
-				.find('.pagination')
 				.find('li')
 				.at(0)
 				.hasClass('disabled')
@@ -159,14 +153,12 @@ describe('pagination', () => {
 
 		expect(
 			wrapper
-				.find('.pagination')
 				.find('li')
 				.at(5)
 				.hasClass('disabled')
 		).toBeTruthy();
 		expect(
 			wrapper
-				.find('.pagination')
 				.find('li')
 				.at(6)
 				.hasClass('disabled')

From da02f39568730c62ae783309667db86d0609ba4d Mon Sep 17 00:00:00 2001
From: Emmanuel Demey <demey.emmanuel@gmail.com>
Date: Tue, 9 Jul 2019 15:33:37 +0200
Subject: [PATCH 3/3] feat: add intigrity control for files

---
 .../operations/document/edition/edition.js    |  1 -
 .../operations/document/edition/validation.js | 35 +++++++++++++++--
 .../document/edition/validation.spec.js       | 39 +++++++++++++++++++
 .../i18n/dictionary/operations/validation.js  |  4 ++
 4 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 src/js/components/operations/document/edition/validation.spec.js

diff --git a/src/js/components/operations/document/edition/edition.js b/src/js/components/operations/document/edition/edition.js
index 7067d3270..aab262ec6 100644
--- a/src/js/components/operations/document/edition/edition.js
+++ b/src/js/components/operations/document/edition/edition.js
@@ -267,7 +267,6 @@ class OperationsDocumentationEdition extends Component {
 										<div
 											{...getRootProps({
 												className: 'dropzone',
-												onDrop: event => event.stopPropagation(),
 											})}
 										>
 											<input
diff --git a/src/js/components/operations/document/edition/validation.js b/src/js/components/operations/document/edition/validation.js
index db7bed55f..064f8a14a 100644
--- a/src/js/components/operations/document/edition/validation.js
+++ b/src/js/components/operations/document/edition/validation.js
@@ -1,8 +1,36 @@
 import D from 'js/i18n';
 import { LINK, DOCUMENT } from '../utils';
 
+/**
+ * Check if a list of document is correct.
+ * This list of document should respect the following criterias
+ * - Should have at least one item
+ * - The name of each files should respect a regexp
+ *
+ * @param {Array<{name}>} files
+ * @returns {{errorMessage: string, fields: { file:  boolean}}}
+ */
+function verifyFile(files = []) {
+	const regexp = /^[a-zA-Z1-9-_\.]*$/;
+	let errorMessage = '';
+
+	if (files.length === 0) {
+		errorMessage = D.requiredFile;
+	} else {
+		const wrongFile = files
+			.map(file => file.name)
+			.find(fileName => !regexp.test(fileName));
+		if (wrongFile) {
+			errorMessage = D.wrongFileName;
+		}
+	}
+	return {
+		errorMessage,
+		fields: { file: errorMessage !== '' },
+	};
+}
+
 export function validate(document, type, files) {
-	console.log(files);
 	const fields = {};
 	let errorMessage = '';
 	if (!document.labelLg1 || !document.labelLg2) {
@@ -15,12 +43,11 @@ export function validate(document, type, files) {
 	} else if (type === LINK && !/https*\:\/\//.test(document.url)) {
 		errorMessage = D.badUrl;
 		fields.url = true;
-	} else if (type === DOCUMENT && files.length === 0) {
-		errorMessage = D.requiredFile;
-		fields.file = true;
 	} else if (!document.lang) {
 		errorMessage = D.requiredLang;
 		fields.lang = true;
+	} else if (type === DOCUMENT) {
+		return verifyFile(files);
 	}
 	return {
 		fields,
diff --git a/src/js/components/operations/document/edition/validation.spec.js b/src/js/components/operations/document/edition/validation.spec.js
new file mode 100644
index 000000000..77186cf4d
--- /dev/null
+++ b/src/js/components/operations/document/edition/validation.spec.js
@@ -0,0 +1,39 @@
+import { validate } from './validation';
+import { DOCUMENT } from '../utils';
+
+describe('validate', () => {
+	describe('documents', () => {
+		const type = DOCUMENT;
+		const document = {
+			labelLg1: '1',
+			labelLg2: '2',
+			lang: 'fr',
+		};
+		it('should return an error if the files is undefined', () => {
+			expect(validate(document, type)).toEqual({
+				errorMessage: 'The file is required',
+				fields: { file: true },
+			});
+		});
+		it('should return an error if the files is an empty array', () => {
+			expect(validate(document, type, [])).toEqual({
+				errorMessage: 'The file is required',
+				fields: { file: true },
+			});
+		});
+		it('should return an error if ta name is not valid', () => {
+			expect(validate(document, type, [{ name: 'name with space' }])).toEqual({
+				errorMessage: 'The name of the file incorrect',
+				fields: { file: true },
+			});
+		});
+		it('should not return any error', () => {
+			expect(
+				validate(document, type, [{ name: 'name_without_space' }])
+			).toEqual({
+				errorMessage: '',
+				fields: { file: false },
+			});
+		});
+	});
+});
diff --git a/src/js/i18n/dictionary/operations/validation.js b/src/js/i18n/dictionary/operations/validation.js
index 95310d7f0..a04e1c6dd 100644
--- a/src/js/i18n/dictionary/operations/validation.js
+++ b/src/js/i18n/dictionary/operations/validation.js
@@ -19,6 +19,10 @@ export default {
 		fr: 'Le fichier est obligatoire',
 		en: 'The file is required',
 	},
+	wrongFileName: {
+		fr: 'Le nom du fichier est incorrect',
+		en: 'The name of the file incorrect',
+	},
 	requiredFamily: {
 		fr: 'La famille est obligatoire',
 		en: 'The family is required',