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

apply change: Fix adding component with non default simple attributes. #67

Conversation

byohay
Copy link
Collaborator

@byohay byohay commented Jun 4, 2022

When adding a component with simple attributes, these attributes might
have default values that aren't nil. Previously, when such a thing
occurred it would result in a MergeError, because the removed change
was assumed to be nil since the component was just added, but in
practice it would be set to the default value.
The solution is to set the removed change value to the default value.

When adding a component with simple attributes, these attributes might
have default values that aren't `nil`. Previously, when such a thing
occurred it would result in a `MergeError`, because the removed change
was assumed to be `nil` since the component was just added, but in
practice it would be set to the default value.
The solution is to set the removed change value to the default value.
@byohay byohay force-pushed the feature/fix-adding-simple-attribute-with-default-value branch from 9283ba7 to 75a5efd Compare June 4, 2022 13:31
Copy link
Member

@ashdnazg ashdnazg left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -694,6 +698,12 @@ def add_attributes_to_component(component, change, change_path, ignore_keys: [])
end
end

def simple_attribute_default_value(component, attribute_name)
component.simple_attributes.find do |attribute|
Copy link
Member

Choose a reason for hiding this comment

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

This is awful, but it's probably not worth using private API

@byohay byohay merged commit f00b748 into Lightricks:main Jun 6, 2022
@byohay byohay deleted the feature/fix-adding-simple-attribute-with-default-value branch June 6, 2022 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants