Merge pull request #123 from ReCodEx/fix-copy-limits

Fix copying limits
ReCodEx · Oct 29, 2017 · 9e39e5b · 9e39e5b
2 parents 9ba120d + 2d99e11
commit 9e39e5b
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 27 deletions.
diff --git a/src/components/forms/EditSimpleLimits/EditSimpleLimits.js b/src/components/forms/EditSimpleLimits/EditSimpleLimits.js
@@ -9,6 +9,15 @@ import styles from './styles.less';
 
 const formName = id => `editEnvironmentSimpleLimits-${id}`;
 
+const fillInDefaultValuesWhereMissing = (testNames, limits) =>
+  testNames.reduce(
+    (acc, test) => ({
+      ...acc,
+      [test]: limits[test] || { memory: 0, 'wall-time': 0 }
+    }),
+    {}
+  );
+
 const EditSimpleLimits = ({
   environments = [],
   editLimits,
@@ -32,18 +41,27 @@ const EditSimpleLimits = ({
             {description} <Label>{platform}</Label>
           </p>
           <ResourceRenderer resource={limits(id)}>
-            {limits =>
-              <EditEnvironmentSimpleLimitsForm
-                {...props}
-                envName={name}
-                config={config.find(forEnv => forEnv.name === id)}
-                initialValues={{ limits }}
-                form={formName(id)}
-                onSubmit={editLimits(id)}
-                setHorizontally={setHorizontally(formName(id), id)}
-                setVertically={setVertically(formName(id), id)}
-                setAll={setAll(formName(id), id)}
-              />}
+            {limits => {
+              const envConfig = config.find(forEnv => forEnv.name === id);
+              return (
+                <EditEnvironmentSimpleLimitsForm
+                  {...props}
+                  envName={name}
+                  config={envConfig}
+                  initialValues={{
+                    limits: fillInDefaultValuesWhereMissing(
+                      envConfig.tests.map(test => test.name),
+                      limits
+                    )
+                  }}
+                  form={formName(id)}
+                  onSubmit={editLimits(id)}
+                  setHorizontally={setHorizontally(formName(id), id)}
+                  setVertically={setVertically(formName(id), id)}
+                  setAll={setAll(formName(id), id)}
+                />
+              );
+            }}
           </ResourceRenderer>
         </div>
       </Col>

diff --git a/src/redux/modules/simpleLimits.js b/src/redux/modules/simpleLimits.js
@@ -57,21 +57,25 @@ const getSimpleLimitsOf = (
   testName
 ) =>
   form[formName].values.limits[testName] ||
-  form[formName].values.limits[testName] ||
+  form[formName].values.initial[testName] ||
   {};
 
-const isSimpleLimitsForm = ({ values }, testName) =>
-  values.limits &&
-  Object.keys(values).length === 1 &&
-  values.limits[testName] &&
-  values.limits[testName].hasOwnProperty('wall-time') &
-    values.limits[testName].hasOwnProperty('memory');
+const isSimpleLimitsForm = ({ registeredFields }, testName) =>
+  registeredFields &&
+  registeredFields.hasOwnProperty(`limits.${testName}.wall-time`) &&
+  registeredFields.hasOwnProperty(`limits.${testName}.memory`);
 
 const getAllSimpleLimitsFormNames = ({ form }, testName) =>
   Object.keys(form).filter(name => isSimpleLimitsForm(form[name], testName));
 
 const getAllTestNames = ({ form }, formName) =>
-  Object.keys(form[formName].values.limits);
+  Object.keys(form[formName].registeredFields)
+    .map(name => {
+      const firstDot = name.indexOf('.');
+      const lastDot = name.lastIndexOf('.');
+      return name.substring(firstDot + 1, lastDot);
+    })
+    .reduce((acc, name) => (acc.indexOf(name) >= 0 ? acc : [...acc, name]), []);
 
 const field = testName => `limits.${testName}`;
 

diff --git a/test/redux/simpleLimits.js b/test/redux/simpleLimits.js
@@ -15,22 +15,46 @@ const exerciseId = random();
 const runtimeEnvironmentId = random();
 const testA = random();
 const testB = random();
+const registeredFields = {
+  [`limits.${testA}.memory`]: {
+    name: `limits.${testA}.memory`,
+    type: 'Field',
+    count: 1
+  },
+  [`limits.${testA}.wall-time`]: {
+    name: `limits.${testA}.wall-time`,
+    type: 'Field',
+    count: 1
+  },
+  [`limits.${testB}.memory`]: {
+    name: `limits.${testB}.memory`,
+    type: 'Field',
+    count: 1
+  },
+  [`limits.${testB}.wall-time`]: {
+    name: `limits.${testB}.wall-time`,
+    type: 'Field',
+    count: 1
+  }
+};
 
 const initialState = {
   form: {
     [formA]: {
+      registeredFields,
       values: {
         limits: {
-          [testA]: { memory: 1, 'wall-time': 2, parallel: 3 },
-          [testB]: { memory: 4, 'wall-time': 5, parallel: 6 }
+          [testA]: { memory: 1, 'wall-time': 2 },
+          [testB]: { memory: 4, 'wall-time': 5 }
         }
       }
     },
     [formB]: {
+      registeredFields,
       values: {
         limits: {
-          [testA]: { memory: 7, 'wall-time': 8, parallel: 9 },
-          [testB]: { memory: 10, 'wall-time': 11, parallel: 12 }
+          [testA]: { memory: 7, 'wall-time': 8 },
+          [testB]: { memory: 10, 'wall-time': 11 }
         }
       }
     }
@@ -42,7 +66,7 @@ const getTestStore = () =>
 
 describe('simpleLimits', () => {
   describe('reducer', () => {
-    it('must copy values horizontally across the same test', () => {
+    it('must copy values horizontally across the same tests', () => {
       const { dispatch, getState } = getTestStore();
 
       setHorizontally(formA, exerciseId, runtimeEnvironmentId, testA)(
@@ -54,6 +78,7 @@ describe('simpleLimits', () => {
         form: {
           [formA]: initialState.form[formA],
           [formB]: {
+            registeredFields,
             values: {
               limits: {
                 [testA]: initialState.form[formA].values.limits[testA],
@@ -65,7 +90,7 @@ describe('simpleLimits', () => {
       });
     });
 
-    it('must copy values vertically across the runtime environment', () => {
+    it('must copy values vertically across the runtime environments', () => {
       const { dispatch, getState } = getTestStore();
 
       setVertically(formA, exerciseId, runtimeEnvironmentId, testA)(
@@ -76,6 +101,7 @@ describe('simpleLimits', () => {
       expect(getState()).to.eql({
         form: {
           [formA]: {
+            registeredFields,
             values: {
               limits: {
                 [testA]: initialState.form[formA].values.limits[testA],
@@ -88,7 +114,7 @@ describe('simpleLimits', () => {
       });
     });
 
-    it('must copy values vertucally across the runtime environment', () => {
+    it('must copy values in all directions across the runtime environments', () => {
       const { dispatch, getState } = getTestStore();
 
       setAll(formA, exerciseId, runtimeEnvironmentId, testA)(
@@ -99,6 +125,7 @@ describe('simpleLimits', () => {
       expect(getState()).to.eql({
         form: {
           [formA]: {
+            registeredFields,
             values: {
               limits: {
                 [testA]: initialState.form[formA].values.limits[testA],
@@ -107,6 +134,7 @@ describe('simpleLimits', () => {
             }
           },
           [formB]: {
+            registeredFields,
             values: {
               limits: {
                 [testA]: initialState.form[formA].values.limits[testA],