Lti (#39)

* Properly handle toggling lti type * Fixes failing specs * Changes status code * Use calculated name
atomicjolt · Feb 22, 2017 · 4da0b39 · 4da0b39
1 parent 7cc9a2f
commit 4da0b39
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 34 deletions.
diff --git a/app/controllers/api/application_instances_controller.rb b/app/controllers/api/application_instances_controller.rb
@@ -29,7 +29,7 @@ def create
 
   def update
     @application_instance.update(application_instance_params)
-    respond_with(@application_instance)
+    render json: @application_instance.as_json(include: :site)
   end
 
   def destroy

diff --git a/client/js/_admin/components/application_instances/form.jsx b/client/js/_admin/components/application_instances/form.jsx
@@ -18,13 +18,13 @@ export const TYPE_RADIOS = {
 export default class Form extends React.Component {
 
   static propTypes = {
-    onChange:       React.PropTypes.func.isRequired,
-    closeModal:     React.PropTypes.func.isRequired,
-    createInstance: React.PropTypes.func.isRequired,
-    newSite:        React.PropTypes.func.isRequired,
-    site_id:        React.PropTypes.string,
-    sites:          React.PropTypes.shape({}),
-    isUpdate:       React.PropTypes.bool,
+    onChange:   React.PropTypes.func.isRequired,
+    closeModal: React.PropTypes.func.isRequired,
+    save:       React.PropTypes.func.isRequired,
+    newSite:    React.PropTypes.func.isRequired,
+    site_id:    React.PropTypes.string,
+    sites:      React.PropTypes.shape({}),
+    isUpdate:   React.PropTypes.bool,
   };
 
   selectSite(option) {
@@ -115,7 +115,7 @@ export default class Form extends React.Component {
         </div>
         <button
           type="button"
-          onClick={() => this.props.createInstance()}
+          onClick={() => this.props.save()}
           className="c-btn c-btn--yellow"
         >
           Save

diff --git a/client/js/_admin/components/application_instances/form.spec.jsx b/client/js/_admin/components/application_instances/form.spec.jsx
@@ -14,7 +14,7 @@ describe('application instance form', () => {
     props = {
       onChange:       () => {},
       closeModal:     () => { modalClosed = true; },
-      createInstance: () => {},
+      save:           () => {},
       newSite:        () => {},
       site_id:        'foo',
       sites:          {},

diff --git a/client/js/_admin/components/application_instances/modal.jsx b/client/js/_admin/components/application_instances/modal.jsx
@@ -58,7 +58,7 @@ export default class Modal extends React.Component {
     });
   }
 
-  createInstance() {
+  save() {
     this.props.save(
       this.props.application.id,
       this.state.newApplicationInstance
@@ -92,7 +92,7 @@ export default class Modal extends React.Component {
         <ApplicationInstanceForm
           {...this.state.newApplicationInstance}
           onChange={(e) => { this.newApplicationInstanceChange(e); }}
-          createInstance={() => this.createInstance()}
+          save={() => this.save()}
           sites={this.props.sites}
           site_id={`${siteId}`}
           closeModal={() => this.closeModal()}

diff --git a/client/js/_admin/components/lti_installs/account_install.jsx b/client/js/_admin/components/lti_installs/account_install.jsx
@@ -36,7 +36,7 @@ export default function AccountInstall(props) {
     tool.consumer_key === props.applicationInstance.lti_key
   ));
   const accountName = props.account ? props.account.name : 'Root';
-  const buttonText = installedTool ? 'Uninstall' : 'Install';
+  const buttonText = `${installedTool ? 'Uninstall from' : 'Install into'} ${accountName}`;
 
   return (
     <div className="c-info">
@@ -47,7 +47,7 @@ export default function AccountInstall(props) {
       <button
         className="c-btn c-btn--yellow"
         onClick={() => install(props.applicationInstance, installedTool)}
-      >{buttonText} at Account Level</button>
+      >{buttonText}</button>
     </div>
   );
 }

diff --git a/client/js/_admin/components/lti_installs/install_pane.jsx b/client/js/_admin/components/lti_installs/install_pane.jsx
@@ -5,7 +5,6 @@ import CourseInstalls from './course_installs';
 import Pagination     from './pagination';
 
 const PAGE_SIZE = 10;
-const ACCOUNT_TYPES = ['basic', 'account_navigation'];
 const COURSE_TYPES = ['basic', 'course_navigation'];
 
 export default class InstallPane extends React.Component {
@@ -77,23 +76,21 @@ export default class InstallPane extends React.Component {
   render() {
     const searchedCourses = this.searchedCourses();
     const pageCount = _.ceil(searchedCourses.length / PAGE_SIZE);
-    let accountInstall = 'Account install not available for this tool';
-    let courseInstalls = 'Course install not available for this tool';
+    let accountInstall = null;
+    let courseInstalls = <p className="c-alert c-alert--info">Course install not available for this tool</p>;
     const {
       applicationInstance,
     } = this.props;
 
     if (applicationInstance) {
-      if (_.includes(ACCOUNT_TYPES, applicationInstance.lti_type)) {
-        accountInstall = (
-          <AccountInstall
-            account={this.props.account}
-            accountInstalls={this.props.account ? this.props.account.installCount : null}
-            applicationInstance={applicationInstance}
-            canvasRequest={this.props.canvasRequest}
-          />
-        );
-      }
+      accountInstall = (
+        <AccountInstall
+          account={this.props.account}
+          accountInstalls={this.props.account ? this.props.account.installCount : null}
+          applicationInstance={applicationInstance}
+          canvasRequest={this.props.canvasRequest}
+        />
+      );
       if (_.includes(COURSE_TYPES, applicationInstance.lti_type)) {
         courseInstalls = (
           <span>

diff --git a/client/js/_admin/components/lti_installs/install_pane.spec.jsx b/client/js/_admin/components/lti_installs/install_pane.spec.jsx
@@ -6,13 +6,15 @@ import InstallPane from './install_pane';
 describe('install pane', () => {
   let result;
 
+  const name = 'the account name';
   const props = {
     canvasRequest: () => {},
     loadingCourses: {},
     applicationInstance: {},
     courses: [],
     account: {
       installCount: 0,
+      name,
     },
     loadExternalTools: () => {},
   };
@@ -29,7 +31,7 @@ describe('install pane', () => {
 
     it('renders the install pane with account installs for basic', () => {
       const button = TestUtils.findRenderedDOMComponentWithTag(result, 'button');
-      expect(button.textContent).toContain('at Account Level');
+      expect(button.textContent).toContain(name);
     });
 
     it('renders the install pane with course installs for basic', () => {
@@ -50,7 +52,7 @@ describe('install pane', () => {
 
     it('renders the install pane with account installs for basic', () => {
       const button = TestUtils.findRenderedDOMComponentWithTag(result, 'button');
-      expect(button.textContent).toContain('at Account Level');
+      expect(button.textContent).toContain(name);
     });
 
     it('renders the install pane without course installs for account_navigation', () => {
@@ -75,9 +77,9 @@ describe('install pane', () => {
       expect(input.placeholder).toContain('Search...');
     });
 
-    it('renders the install pane without account installs for course_navigation', () => {
+    it('renders the install pane with account installs for course_navigation', () => {
       const button = TestUtils.scryRenderedDOMComponentsWithTag(result, 'button');
-      expect(button.length).toBe(0);
+      expect(button.length).toBe(1);
     });
   });
 });
diff --git a/client/js/_admin/reducers/application_instances.js b/client/js/_admin/reducers/application_instances.js
@@ -20,6 +20,7 @@ export default function instances(state = initialState, action) {
       return newState;
     }
 
+    case ApplicationInstancesConstants.SAVE_APPLICATION_INSTANCE_DONE:
     case ApplicationInstancesConstants.CREATE_APPLICATION_INSTANCE_DONE: {
       const newState = _.cloneDeep(state);
       newState[action.payload.id] = action.payload;

diff --git a/client/styles/admin/partials/_alerts.scss b/client/styles/admin/partials/_alerts.scss
@@ -5,9 +5,13 @@
 }
 
 .c-alert--success{
-  color: #00FF00;
+  color: $green;
+}
+
+.c-alert--info{
+  color: $yellow;
 }
 
 .c-alert--danger{
-  color: #FF0000;
+  color: $red;
 }
diff --git a/spec/controllers/api/application_instances_controller_spec.rb b/spec/controllers/api/application_instances_controller_spec.rb
@@ -70,7 +70,7 @@
               lti_secret: "12345",
             },
             format: :json
-        expect(response).to have_http_status(204)
+        expect(response).to have_http_status(200)
       end
     end