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

Add failing test for nested properties with booleans #415

Conversation

josemarluedke
Copy link
Contributor

This adds a test for a bug I encountered today.

cc/ @snewcomer

@josemarluedke josemarluedke force-pushed the add-failing-test-for-checkbox branch from 6ae74c4 to 24e6e0d Compare January 20, 2020 01:25
@josemarluedke josemarluedke force-pushed the add-failing-test-for-checkbox branch from 24e6e0d to f4ec285 Compare January 20, 2020 02:01
@snewcomer
Copy link
Collaborator

@josemarluedke I think I know what we can do. We check if the old content value is equal to what we want to set and if so, don't do anything. I'll see if we can refactor this.

@snewcomer
Copy link
Collaborator

#416 partially fixed your issues. I think there might be something else we need to do!

@snewcomer snewcomer mentioned this pull request Jan 20, 2020
@snewcomer
Copy link
Collaborator

snewcomer commented Jan 20, 2020

Ok everything should be g2g on master! Can't thank you enough for your contributions. These were also issues in 2.x and 1.x so these are very nice additions! Feel free to merge in master and we can get this PR merged!

@josemarluedke josemarluedke force-pushed the add-failing-test-for-checkbox branch from f4ec285 to ddd15b5 Compare January 20, 2020 16:30
@josemarluedke
Copy link
Contributor Author

josemarluedke commented Jan 20, 2020

Rebased with master, tests did pass locally. Thank you @snewcomer for working on fixing this.

Should we get these tests merged anyway?

@snewcomer snewcomer merged commit d50e368 into adopted-ember-addons:master Jan 20, 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