Add JSON Support

[lsmith77]

fix for #82

note I am not so expierienced with developing for laravel .. it seems to work ok on a quick check.
I need to add tests. aside from this there are some "taste" things to decide .. like using the ternary syntax inside the Version class or if the call to config() should be "cached" in case multiple versionable models are handled in one request.

[codecov-commenter]

Codecov Report

Merging #83 (046814a) into master (043b311) will decrease coverage by 16.25%.
The diff coverage is 14.28%.

❗ Current head 046814a differs from pull request most recent head 599fa0c. Consider uploading reports for the commit 599fa0c to get more accurate results

@@ Coverage Diff @@## master #83 +/- ##
=============================================
- Coverage 98.31% 82.06% -16.26% - Complexity 47 56 +9 
=============================================
Files 3 4 +1 Lines 119 145 +26 =============================================
+ Hits 117 119 +2 - Misses 2 26 +24 

Impacted Files	Coverage Δ
...rc/Mpociot/Versionable/Console/ConvertEncoding.php	0.00% <0.00%> (ø)
.../Mpociot/Versionable/Providers/ServiceProvider.php	100.00% <100.00%> (ø)
src/Mpociot/Versionable/Version.php	94.11% <100.00%> (ø)
src/Mpociot/Versionable/VersionableTrait.php	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 043b311...599fa0c. Read the comment docs.

[lsmith77]

@nonoesp any feedback here? thx

[nonoesp]

Hey, @lsmith77 — I'll try to take a look at this soon (and hopefully test it).

Thanks for the work!

[nonoesp]

Hi, @lsmith77 - I have little time to review and test this.

I'd really appreciate if you could describe the steps I should follow to test this pull request and verify everything works?

🙏

[lsmith77]

I stilll owe this PR tests.

but fundamentally you have to make sure Mpociot\Versionable\Providers\ServiceProvider::class is registered, that you set in the config the encoding to 'encoding' => 'json', and then run php artisan versionable:convert-encoding to convert from serialize to json` encoding.

[lsmith77]

I would appreciate advice on how to best test this. f.e. should I use Config::set() in the tests?

[lsmith77]

I am also wondering if instead or in addition to the command, there should be a migration?

[nonoesp]

Hey! Shouldn't this be $version->model_data = $this->getEncoding() == 'json' ? json_encode($this->attributesToArray()) : serialize($this->attributesToArray());?

[lsmith77]

indeed! .. goes to show that this PR needs tests

[nonoesp]

Yes, it does! I was testing manually yesterday and it doesn't seem to quite work with my site.

[lsmith77]

@nonoesp I think I fixed the bug you encountered, however still didn't have time to figure out how to add tests and generate an efficient migration script. hopefully this week I will manage to do it.

[lsmith77]

@nonoesp I hope this is now ready to be merged.

I added a very simple test case and also documented the process to create a migration with lazy loading (which however requires laravel 8).

[nonoesp]

Hi, @lsmith77 - I'll be taking a look at this!

[lsmith77]

ping?

[nonoesp]

Thanks for the reminder!

[nonoesp]

Hi @lsmith77 can you resolve the conflicts?

[lsmith77]

done