Merge pull request #201 from anehx/analysis-edit-states

Fix states in bulk edit mask

Author: sliverc

app/analysis/edit/controller.js

@@ -7,6 +7,7 @@ import { cleanParams } from 'timed/utils/url'
 import computed from 'ember-computed-decorators'
 import { toQueryString } from 'timed/utils/url'
 import IntersectionValidations from 'timed/validations/intersection'
+import { later } from '@ember/runloop'
 import {
   underscoreQueryParams,
   serializeParachuteQueryParams,
@@ -154,7 +155,12 @@ export default Controller.extend(AnalysisEditQueryParams.Mixin, {
     },
 
     reset(changeset) {
-      changeset.rollback()
+      // We have to defer the rollback for some milliseconds since the combobox
+      // reset action triggers mutation of customer, task, and project which
+      // would be run after this rollback and therefore trigger changes
+      later(() => {
+        changeset.rollback()
+      })
     }
   }
 })

app/analysis/edit/template.hbs

@@ -34,21 +34,24 @@
                   <div class="form-group" data-test-customer>
                     <label>
                       Customer
-                      {{#unless f.model.customer}}{{not-identical-warning}}{{/unless}}
+                      {{#unless model.customer}}{{not-identical-warning}}{{/unless}}
+                      {{#unless (eq f.model.change.customer.id model.customer.id)}}{{changed-warning}}{{/unless}}
                     </label>
                     {{t.customer}}
                   </div>
                   <div class="form-group" data-test-project>
                     <label>
                       Project
-                      {{#unless f.model.project}}{{not-identical-warning}}{{/unless}}
+                      {{#unless model.project}}{{not-identical-warning}}{{/unless}}
+                      {{#unless (eq f.model.change.project.id model.project.id)}}{{changed-warning}}{{/unless}}
                     </label>
                     {{t.project}}
                   </div>
                   <div class="form-group" data-test-task>
                     <label>
                       Task
-                      {{#unless f.model.task}}{{not-identical-warning}}{{/unless}}
+                      {{#unless model.task}}{{not-identical-warning}}{{/unless}}
+                      {{#unless (eq f.model.change.task.id model.task.id)}}{{changed-warning}}{{/unless}}
                     </label>
                     {{t.task}}
                   </div>
@@ -57,30 +60,34 @@
                 {{#f.input data-test-comment name='comment' as |fi|}}
                   <label>
                     Comment
-                    {{#if (eq f.model.comment null)}}{{not-identical-warning}}{{/if}}
+                    {{#if (eq model.comment null)}}{{not-identical-warning}}{{/if}}
+                    {{#unless (eq f.model.comment model.comment)}}{{changed-warning}}{{/unless}}
                   </label>
                   <input type="text" placeholder="Enter comment..." class="form-control" onchange={{action fi.update value='target.value'}} value={{fi.value}}>
                 {{/f.input}}
 
                 {{#f.input data-test-not-billable name='notBillable' as |fi|}}
                   {{#sy-checkbox checked=fi.value on-change=fi.update}}
                     Not billable
-                    {{#if (eq f.model.notBillable null)}}{{not-identical-warning}}{{/if}}
+                    {{#if (eq model.notBillable null)}}{{not-identical-warning}}{{/if}}
+                    {{#unless (eq f.model.notBillable model.notBillable)}}{{changed-warning}}{{/unless}}
                   {{/sy-checkbox}}
                 {{/f.input}}
 
                 {{#f.input data-test-review name='review' as |fi|}}
                   {{#sy-checkbox checked=fi.value on-change=fi.update}}
                     Needs Review
-                    {{#if (eq f.model.review null)}}{{not-identical-warning}}{{/if}}
+                    {{#if (eq model.review null)}}{{not-identical-warning}}{{/if}}
+                    {{#unless (eq f.model.review model.review)}}{{changed-warning}}{{/unless}}
                   {{/sy-checkbox}}
                 {{/f.input}}
 
                 {{#if (or (eq reviewer session.data.user.id) session.data.user.isSuperuser)}}
                   {{#f.input data-test-verified name='verified' as |fi|}}
                     {{#sy-checkbox checked=fi.value on-change=fi.update}}
                       Verfied
-                      {{#if (eq f.model.verified null)}}{{not-identical-warning}}{{/if}}
+                      {{#if (eq model.verified null)}}{{not-identical-warning}}{{/if}}
+                      {{#unless (eq f.model.verified model.verified)}}{{changed-warning}}{{/unless}}
                     {{/sy-checkbox}}
                   {{/f.input}}
                 {{/if}}

app/components/changed-warning/component.js

@@ -0,0 +1,8 @@
+import Component from '@ember/component'
+
+export default Component.extend({
+  tagName: 'span',
+  classNames: ['color-warning'],
+  attributeBindings: ['title'],
+  title: 'This field has unsaved changes'
+})

app/components/changed-warning/template.hbs

@@ -0,0 +1 @@
+ {{fa-icon 'warning'}}

app/components/task-selection/component.js

@@ -226,7 +226,7 @@ export default Component.extend({
       this.set('_project', value)
 
       if (value && value.get('customer')) {
-        this.set('_customer', value.get('customer'))
+        this.set('customer', value.get('customer'))
       }
 
       /* istanbul ignore else */
@@ -262,10 +262,7 @@ export default Component.extend({
       this.set('_task', value)
 
       if (value && value.get('project')) {
-        this.setProperties({
-          _project: value.get('project'),
-          _customer: value.get('project.customer')
-        })
+        this.set('project', value.get('project'))
       }
 
       later(this, () => {

app/validations/intersection.js

@@ -1,5 +1,7 @@
 import validateIntersectionTask from 'timed/validators/intersection-task'
+import validateNullOrNotBlank from 'timed/validators/null-or-not-blank'
 
 export default {
-  task: validateIntersectionTask()
+  task: validateIntersectionTask(),
+  comment: validateNullOrNotBlank()
 }

app/validators/null-or-not-blank.js

@@ -0,0 +1,10 @@
+import { capitalize } from '@ember/string'
+
+export default function validateNullOrNotBlank() {
+  return (key, newValue) => {
+    return (
+      !!(newValue === null || (newValue && newValue.length > 0)) ||
+      `${capitalize(key)} must be null or not blank`
+    )
+  }
+}

tests/acceptance/analysis-edit-test.js

@@ -51,15 +51,17 @@ describe('Acceptance | analysis edit', function() {
     let { data: { type, attributes, relationships } } = res
 
     expect(type).to.equal('report-bulks')
+
     // only changed attributes were sent
     expect(Object.keys(attributes)).to.deep.equal([
       'comment',
       'not-billable',
       'review'
     ])
-    // since no relationships were edited - nothing was sent
     expect(Object.keys(relationships)).to.deep.equal([
-      'task' // task will be changed since the relation updates..
+      'customer',
+      'project',
+      'task'
     ])
 
     expect(currentURL()).to.equal('/analysis')

tests/integration/components/changed-warning/component-test.js

@@ -0,0 +1,17 @@
+import { expect } from 'chai'
+import { describe, it } from 'mocha'
+import { setupComponentTest } from 'ember-mocha'
+import hbs from 'htmlbars-inline-precompile'
+import { find } from 'ember-native-dom-helpers'
+
+describe('Integration | Component | changed warning', function() {
+  setupComponentTest('changed-warning', {
+    integration: true
+  })
+
+  it('renders', function() {
+    this.render(hbs`{{changed-warning}}`)
+
+    expect(find('i.fa.fa-warning')).to.be.ok
+  })
+})

tests/unit/validators/null-or-not-blank-test.js

@@ -0,0 +1,11 @@
+import { expect } from 'chai'
+import { describe, it } from 'mocha'
+import validateNullOrNotBlank from 'timed/validators/null-or-not-blank'
+
+describe('Unit | Validator | null or not blank', function() {
+  it('works', function() {
+    expect(validateNullOrNotBlank()('key', 'test')).to.be.true
+    expect(validateNullOrNotBlank()('key', null)).to.be.true
+    expect(validateNullOrNotBlank()('key', '')).to.be.a('string')
+  })
+})