-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add configs to application and application_instance #51
Conversation
bfcoder
commented
Mar 9, 2017
•
edited
Loading
edited
- Add configs to application and application_instance in the database
- Add serializer for configs
- Can now access model.foo instead of model.config[:foo]
- Ensure new app instance copies the default config from an app if it is blank
- Add example to seeds
- Add UI to edit default config for applications
- Add UI to edit config for app instance
- Add/update specs
Can now access model.foo instead of model.config[:foo] Ensure new app instance copies the default config from an app if it is blank Add example to seeds
@@ -22,12 +22,14 @@ def show | |||
def create | |||
@application_instance.domain = | |||
"#{@application_instance.lti_key}.#{ENV['APP_URL']}" | |||
@application_instance.config = params[:application_instance][:config] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a note about why we have to use the param directly instead of using strong params.
app/models/application_instance.rb
Outdated
@@ -12,11 +13,15 @@ class ApplicationInstance < ActiveRecord::Base | |||
errors.add(:lti_key, "cannot be changed after creation") if lti_key_changed? | |||
end | |||
|
|||
# example store_accessor for config | |||
store_accessor :config, :foo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment this out and add more instruction on what it's for.
Show warning for bad JSON in config Fix variables in textarea component Explicitly use each prop in Textarea component use camelCase for defaultConfig
@@ -8,6 +8,7 @@ export default class Modal extends React.Component { | |||
application: React.PropTypes.shape({ | |||
name: React.PropTypes.string, | |||
description: React.PropTypes.string, | |||
default_config: React.PropTypes.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'application.default_config' PropType is defined but prop is never used react/no-unused-prop-types
@@ -11,6 +11,7 @@ export default class Modal extends React.Component { | |||
save: React.PropTypes.func.isRequired, | |||
applicationInstance: React.PropTypes.shape({ | |||
id: React.PropTypes.number, | |||
config: React.PropTypes.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'applicationInstance.config' PropType is defined but prop is never used react/no-unused-prop-types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM