@kevinansfield I have a thought on how to fix this, will try implementing it and see how it goes.

kevin, I changed around the service to be more promise-based rather than CP based; however, the same error is still occurring in tests for me locally (settings adapter#save is still not being called 😞)

@kevinansfield the tests passed locally so this should be good to go 👍

🎊

Well, it appears it works locally because I'm using phantomjs 2.1 locally, but it fails using 1.9.8 on Travis. Thoughts? (It works using chrome testing as well)

It seems to be due to phantomjs not picking up the changes to the feature checkbox on change

Maybe worth looking at merging #6051 before spending too much time trying to find workarounds for old phantomjs versions.

@kevinansfield is there anything else I need to do for this? I feel like the other two stubbed tests can be filled out as a part of #6431

As it is right now the settings.save().catch() is broken, I think we need to detect whether we get passed any errors or if the underlying settings model has validation errors and react accordingly. I think that should be fairly simple fix and will enable the pending tests to be filled out.

#6431 is more about improving the behaviour so that we don't need workarounds like the above repeated throughout the codebase - we should definitely have working examples with tests where possible before working on it in order to validate the refactoring.

@kevinansfield I fixed the error handling and filled out the remaining tests; however, I'm not sure how/if it's possible to squash my two commits down into one b/c of the order in which the changes occurred. If there is a way, though, I'll gladly squash them 😄

Finally, it's good to go 👍