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.

Sign up for GitHub

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

Jump to bottom

Records cannot be added to an empty ActiveJSON datastore #203

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Records cannot be added to an empty ActiveJSON datastore #203

wants to merge 4 commits into from
+19 −10

Commits on Apr 6, 2022

  1. Fix spec label

    @davidstosik
    davidstosik committed Apr 6, 2022
    Configuration menu
    Copy the full SHA
    547bcc5 View commit details
    Browse the repository at this point in the history

  2. Write a spec that triggers the bug I'm trying to fix 

    The expectation is that the code won't trigger an exception, but it
does:

    ActiveJSON::Base.create does not fail when the loaded JSON was empty
    Failure/Error: Empty.create()
    NoMethodError:
      undefined method `length' for nil:NilClass
    # ./lib/active_hash/base.rb:135:in `insert'
    # ./lib/active_hash/base.rb:497:in `save'
    # ./lib/active_hash/base.rb:174:in `create'
    # ./spec/active_json/base_spec.rb:51:in `block (3 levels) in <top (required)>'
    @davidstosik
    davidstosik committed Apr 6, 2022
    Configuration menu
    Copy the full SHA
    3db1f79 View commit details
    Browse the repository at this point in the history

  3. Fix bug where records can't be added to an originally empty ActiveJSO… 

    …N datastore

Simple fix would be to replace `@records = nil` with `@records = []`,
but the suggested approach as a better impact on the code base, avoiding
repetitions such as `@records || []`.
    @davidstosik
    davidstosik committed Apr 6, 2022
    Configuration menu
    Copy the full SHA
    5312cbb View commit details
    Browse the repository at this point in the history

  4. Make use of delete_all in data= for clarity

    @davidstosik
    davidstosik committed Apr 6, 2022
    Configuration menu
    Copy the full SHA
    4ccff1a View commit details
    Browse the repository at this point in the history