j42 / laravel-firebase

A Firebase port for Laravel (4.2+)
MIT License
51 stars 22 forks source link

Whitelisting Models To Sync #2

Closed DavidPhilip closed 10 years ago

DavidPhilip commented 10 years ago

I'd like to keep the sync option on while only syncing models I want to. I don't think this is possible right, according to the docs you can only whitelist properties and listing none syncs everything anyway.

j42 commented 10 years ago

David,

Apologies if the documentation wasn't very clear... I know I originally had that set as a default.

The Access Tokens section actually contains the configuration you would put in app/config/database.php, and if you set 'sync' => false it will disable the automatic syncing.

https://github.com/j42/laravel-firebase#access-tokens

This way you can add the whitelist only to the models you would want to sync. Can you think of any situations where you would want more configurability here? Is it worth abstracting this to a package configuration setting?

DavidPhilip commented 10 years ago

Oh damn, I didn't see that :)

It's a detail but from my point of view it's kinda inconvenient to turn on sync for some models by whitelisting their properties in case I want to sync all of them. At the same time 'sync' => false in the config which is kinda paradox. Maybe name it to auto_sync or something that describes the behavior better? Also a property for models like $firebase_sync = true; would be great or like I already proposed you could whitelist classnames which would be a more central approach. I'm not sure which one I like better, what do you think?

j42 commented 10 years ago

Hmm seems like the issue is poor documentation on my part... I believe the use cases you are describing are supported:

sync => false + whitelist = if a model has a whitelist those properties will be pushed on the Eloquent update/save events no matter what (whitelist overrides)

sync => true + whitelist = all models auto-sync all parameters unless a whitelist is set, in which case, only the whitelisted parameters are pushed

sync => false + no whitelist = nothing is synced

sync => true + no whitelist = everything is synced

Not sure it's necessary to have a classmap, unless I'm missing an edge case? My goal here is to implement it in a similar way to $fillable or $appends at the property-level.

DavidPhilip commented 10 years ago

Of course they are supported, got that :) I just think that if I turn off sync because I don't want to sync models from vendor-packages etc. it could be faster to set up the models to override the sync by setting a boolean as an option or a wildcard in the array. Of course as it is now it just has to be done once but it could get kinda messy in big projects maybe?

Anyways this package is very useful for me and I want to thank you for your effort!

DavidPhilip commented 10 years ago

Amazing commitment :) Thumbs up, I'll try to contribute if the chance arises!