Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 19 additions & 1 deletion src/helpers/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,22 @@ export const compareEmail = (email1, email2, options = { UNIQUE_GMAIL_VALIDATION
*/
export const compareHandles = (handle1, handle2) => (
(handle1 || '').toLowerCase() === (handle2 || '').toLowerCase()
)
)

// remove empty object, null/undefined value and empty string recursively from passed obj
const deepClean = obj => _.transform(obj, (result, value, key) => {
const isCollection = _.isObject(value)
const cleaned = isCollection ? deepClean(value) : value
// exclude if empty object, null, undefined or empty string
if ((isCollection && _.isEmpty(cleaned)) || _.isNil(value) || value === '') {
return
}
_.isArray(result) ? result.push(cleaned) : (result[key] = cleaned)
})

/**
* Helper method to clean given object from null, undefined or empty property.
*
* @param {Object} obj the object to clean
*/
export const clean = obj => _.isObject(obj) ? deepClean(obj) : obj
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
STEP_VISIBILITY,
STEP_STATE,
} from '../../../../helpers/wizardHelper'
import { clean } from '../../../../helpers/utils'

import './EditProjectForm.scss'

Expand Down Expand Up @@ -151,6 +152,17 @@ class EditProjectForm extends Component {
template: updatedTemplate,
project: hidedSomeNodes ? nextProps.project : this.state.project,
})

// re-check again if any hidden values when an option is deselected
const updatedProject = clean(removeValuesOfHiddenNodes(updatedTemplate, nextProps.project))
const skipProperties = ['members', 'invites']
const clearUpdatedProject = clean(_.omit(updatedProject, [...skipProperties, 'isDirty']))
const clearUpdatedNonDirtyProject = clean(_.omit(nextProps.projectNonDirty, skipProperties))
const isDirty = !_.isEqual(clearUpdatedProject, clearUpdatedNonDirtyProject)
// update the state, always use this flag to check if changed
this.setState({
isProjectDirty: isDirty,
})
}
}
}
Expand Down Expand Up @@ -206,7 +218,7 @@ class EditProjectForm extends Component {
}

isChanged() {
return !!this.props.project.isDirty
return !!this.state.isProjectDirty
Copy link
Collaborator

@maxceem maxceem Jul 16, 2019

Choose a reason for hiding this comment

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

@gets0ul I guess when we are making this change, we don't use isDirty which we update inside Redux store, so I have a couple of questions:

  1. Should we still have and update isDirty in the Redux store?
  2. Do you think it's safe to use isProjectDirty instead of isDirty from the Redux store?

Copy link
Contributor Author

@gets0ul gets0ul Jul 16, 2019

Choose a reason for hiding this comment

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

isDirty from Redux is still used.
That is where the initial value of isProjectDirty come from.

Both will always have same value most of the time.
The only time they can be different is for the cases I already mentioned here #3175 (comment)

When a node is updated and the hidden value is still there.
When an option is selected and the default value is set.

isDirty will stay true when that option is deselected because of that hidden default value.
That's the only time, the value of isProjectDirty will be different.

I think it is safe as we have already been using the state isProjectDirty when creating SpecSection component to render the questions.

}

enableButton() {
Expand Down
12 changes: 9 additions & 3 deletions src/projects/reducers/project.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
} from '../../config/constants'
import _ from 'lodash'
import update from 'react-addons-update'
import { clean } from '../../helpers/utils'

const initialState = {
isLoading: true,
Expand Down Expand Up @@ -631,13 +632,18 @@ export const projectState = function (state=initialState, action) {
if (key === 'screens' || key === 'features' || key === 'capabilities') {
return srcValue// srcValue contains the changed values from action payload
}

// project's name might contain ampersand
if (key === 'name') {
return _.escape(srcValue)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gets0ul I'm not sure what is the reason for adding this code, is there any situation when it helps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxceem

It is for the case like this https://connect.topcoder-dev.com/projects/7977/scope
when a project contains ampersand on it's name.

}
)
// dont' compare this properties as they could be not added to `projectNonDirty`
// or mutated somewhere in the app
const skipProperties = ['members']
const clearUpdatedProject = _.omit(updatedProject, skipProperties)
const clearUpdatedNonDirtyProject = _.omit(state.projectNonDirty, skipProperties)
const skipProperties = ['members', 'invites']
const clearUpdatedProject = clean(_.omit(updatedProject, skipProperties))
const clearUpdatedNonDirtyProject = clean(_.omit(state.projectNonDirty, skipProperties))
if (!_.isEqual(clearUpdatedProject, clearUpdatedNonDirtyProject)) {
updatedProject.isDirty = true
}
Expand Down