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.

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

BUGFIX: Ignores non-configurable properties when associating existing collection item GUIDs #504

Conversation

mrdavidlaing
Copy link

Fixes a bug (related to #207) where new GUIDs were being assigned for collection items that contained non-configurable properties even though non of the values had changed

@cf-gitbot
Copy link
Member

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

@@ -115,6 +115,9 @@ func (item responsePropertyCollectionItem) getFieldValuesExceptGUID() map[interf
if key == "guid" {
continue
}
if !valueObj.(map[interface{}]interface{})["configurable"].(bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In terms of readability, the only think I would change is pulling out !valueObj.(map[interface{}]interface{})["configurable"].(bool) into

isNotConfigurable = valueObj.(map[interface{}]interface{})["configurable"].(bool)
if isNotConfigurable {
  continue
}

Copy link
Author

Choose a reason for hiding this comment

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

Implemented.

@kcboyle
Copy link
Contributor

kcboyle commented Aug 27, 2020

I think this looks great. I'd be happy to pull it in. CI failures are due to things not in your control

@mrdavidlaing mrdavidlaing force-pushed the ignore_non_configurable_properties_in_collections_when_associating_guids branch from 383878d to 58f67d0 Compare August 27, 2020 17:22
@kcboyle kcboyle merged commit 3b51292 into pivotal-cf:main Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants