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

#114 Add support for default and overridden settings #152

Closed
wants to merge 1 commit into from

Conversation

rjcorwin
Copy link
Contributor

@rjcorwin rjcorwin commented Aug 30, 2016

Now when calling Tangerine.settings.get('foo'), if there is a foo property in the configuration document, that property will be returned. This PR does this by overriding the sync method for the Settings Class. On Settings.fetch(), the sync method will get called and it will first get the settings doc, then get the configuration doc, apply configuration doc over settings doc, and then finish.

Still to do is to include the configuration doc when compiling an APK and then port this from client to editor so it behaves the same way.

@rjcorwin rjcorwin changed the title #114 Allow the configuration doc to override the settings doc in client #114 Allow the configuration doc to override the settings doc Aug 30, 2016
@rjcorwin rjcorwin self-assigned this Sep 2, 2016
@rjcorwin rjcorwin added ready and removed in progress labels Sep 6, 2016
@mfinholt mfinholt changed the title #114 Allow the configuration doc to override the settings doc #114 Add support for default and overridden settings Sep 12, 2016
@mfinholt
Copy link
Member

mfinholt commented Sep 12, 2016

While the intent of this PR is the same as defined above, the implementation has been refined upon further discussion.

The overarching goal is to have an overridable settings definition structure. Rather than confusing things with the various configuration and settings documents, we will be moving forward with the following documents:

  • "settings-default": settings-default is responsible for holding any and all default application settings. This includes those defined and pushed by the server on init/update of a TSI that include network topology definition.
  • "settings": the settings document is responsible for overriding or extending any variables defined in the settings-default document.
    NOTE: the settings document is defined by the collection rather than the document name. This is done to allow for a more modular approach to settings file management
  • Configure build-scripts to build settings-default rather than settings
  • Configure group creation to copy settings-default rather than settings
  • Create SettingsManager class to handle settings crud actions
  • Refactor current Settings model to be a collection
  • Document use of config doc and refactor into settings-default

@mfinholt mfinholt added this to the Tutor Integration milestone Oct 3, 2016
@mfinholt mfinholt added in progress and removed ready labels Oct 3, 2016
@rjcorwin rjcorwin removed this from the Tutor Integration milestone Mar 2, 2017
@rjcorwin
Copy link
Contributor Author

Revisiting this with fresh eyes, it occurred to me our proposed solutions have been very couchapp influenced. What I think we might need is to stop thinking about Documents and think more how best to implement Singletons for Settings and distinguishing Environment Variables as something different. That said, we have an immediate need to be able to push updates to Views and cannot do so because doing so would overwrite customizations to settings documents. We could immediately move the ./editor/app/_docs folder to ./default-group-docs and then in our entrypoint curl each of the JSON files in that folder into the tangerine database so they are there when a new group is created. Below is a write up of my ideas for a long term solution.

A settings pattern using Singleton Classes includes:

  • A superclass Settings Class that other types of settings can use to create Settings subclasses.
  • Settings subclasses are used as Singletons, you only instantiate them once.
  • Defaults for Settings subclasses are stored on the corresponding subclass declaration.
  • Overrides for Settings subclasses are stored in the database (translate to one doc per Settings subclass in a Document Database).
  • Environment Variables is not a Settings subclasses, however they it is a Singleton but the application may not override it.

What this means for our current implementation is that the default docs in ./editor/app/_docs are moving to defaults property in the Settings subclasses which extend Backbone.Model. The only time you'll see one of those default docs in the database is when it is overidden.

Some things we would need to do to implement this:

  • Extract settings in the settings doc out into Settings subclasses like GroupSettings and WorkflowSettings.
  • Remove ./editor/app/_docs and move them to defaults in their Settings subclasses.
  • Create an Environment Class to be used as a Singleton that always gets its data from a backend endpoint.
  • Refactor any code that might need to be refactored due to the new way of getting a setting.

@rjcorwin
Copy link
Contributor Author

@mfinholt I opened a new ticket detailing my proposal in from the last comment. I think the title of the ticket capture the essence but I also added an "Important differences" section which I think is worth a read.

Define Group settings defaults in Singleton Classes as opposed to in CouchApp docs and extract out Environment Variables API #249

@rjcorwin
Copy link
Contributor Author

Priority is on v3. Closing.

@rjcorwin rjcorwin closed this Sep 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants