CSC/ECE 517 Fall 2020 - E2063. Refactor tree-display.js and tree display controller.rb

E2063. Refactoring the tree-display.js and tree display controller.rb

This page provides a description of the Expertiza based OSS project.

About Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

Background

The tree-display.js and its tree_display_controller.rb files are designed to allow Expertiza users to view their Assignments, Courses and Questionnaires at one place. This is the primary control page, as well as the home page for instructors on Expertiza which allows them to create, modify, delete and view Assignments.

The primary problem with this is that both the files, due to their bulky and unoptimized methods, slow the rendering of UI on screen. The methods in these files can be studied and refactored to improve the overall performance of this controller and its corresponding UI. Moreover, any obsolete or unused methods can be removed and DRY principle should be implemented. This project mostly revolves around these 2 files, and would involve refactoring JavaScript more than Ruby on Rails. Knowledge of JavaScript is a prerequisite for this project.

Accomplishments

The following tasks were accomplished in this project:

• Removed methods that were not used
• Modified tree display controller to better parse data prior to being accessed by the react client
• Fixed beta branch issue with showing dropdowns for courses and questionnaires.
• Fixed beta branch issue with showing edit option for courses and assignments.

About Tree Display Controller

This class manages different the different tabs: courses, assignments, and questionaires.

Current Implementation

Functionality

• Instructors can view the course, assignment and questionnaire tabs. Instructors can select rows for questionnaire's and courses to view a dropdown of more information. For course rows, they have options to edit, delete, copy, add TA, create an assignment, create teams, view grades, assign surveys, and view reviews. For assignments, they can edit, delete, copy, assign to course, create teams, assign reviewers, view submissions, view scores, view reports, and view survey responses.

Drawbacks and Solutions

• Problem 1: The dropdown doesn't work for assignments and questionaires. This is due to the handleExpandClick function, as it appears that the get_sub_folder_contents method is not capable of returning the data.

if(this.props.dataType!='assignment') {
            _this = this;
            jQuery.get('/tree_display/get_sub_folder_contents',
                {
                    reactParams2: newParams
                },
                function (data) {
                    _this.props.data[id.split("_")[2]]['children'] = data;
                    _this.forceUpdate();
                }, 'json');
        }

• Solution: Change the the JQuery route to a post because you can't give parameters to a get route and also change this in routes.rb file.

jQuery.post('/tree_display/get_sub_folder_contents',
                {
                    reactParams2: newParams
                },

                
post :get_sub_folder_contents

• Problem 2: Tree display controller needs to better parse the data prior to being access to the react client because currently, there are too many comparisons when checking for the type of tab.

Example:

var SimpleTableRow = React.createClass({
    render: function () {
      var creation_date;
      var updated_date;
      var colWidthArray = ["30%", "0%", "0%", "0%", "25%", "25%", "20%"]
      var colDisplayStyle = {
        "display": ""
      }
      if (this.props.dataType === 'questionnaire') {
        colWidthArray = ["30%", "0%", "0%", "0%", "20%", "20%", "30%"]
        colDisplayStyle = {
          "display": "none"
        }
      } else if(this.props.dataType === 'course') {
        colWidthArray = ["20%", "0%", "0%", "20%", "20%", "20%", "20%"]
      }
     ......

}

• Solution: Centralize the attributes of each tab so that the tree_display doesn't need to keep parsing between the tabs.

• Problem 3: The Assignments and Courses tab isn't showing the edit option for each row. It seems that what's implemented is trying to verify that there was an active user and the active user is null.

if (app_variables.currentUserId == null || this.props.instructor_id == app_variables.currentUserId) {
              moreContent.push(
                  <span>
                <a title="Edit" href={"/" + newNodeType + "/" + (parseInt(this.props.id) / 2).toString() + "/edit"}><img
                    src="/assets/tree_view/edit-icon-24.png"/></a>
              </span>
              );
          }

• Solution: Remove the active user implementation and replace with a check to make sure the current tab isn't Questionnaire.

if(newNodeType != 'questionnaires') { // should only be viewed by either assignments or courses
              moreContent.push(
                  <span>
                <a title="Edit" href={"/" + newNodeType + "/" + (parseInt(this.props.id) / 2).toString() + "/edit"}><img
                    src="/assets/tree_view/edit-icon-24.png"/></a>
              </span>
              );
          }

• Drawback:The assignments tab did not have any data when we used the reset debugging extension (i.e. the childNodes attribute was null). Thus we assumed there was no data to display underneath each assignment.

-After debugging, we found that the "nodeType" attribute in the :reactParams field of the post request identifies the type of childNodes to be retrieved (i.e. "Courses" or "Questionnaires")

-We found that a "FolderNode" value for this attribute equates to a questionnaire.

New Implementation

Consolidation

A lot of the efforts in pushing objects into moreContent could be consolidated into a central format. An example of how its currently implemented for courses is:

if (newNodeType === 'course') {
          moreContent.push(
            <br/>
          )
          if (this.props.is_available) {
            moreContent.push(
              <span>
                <a title="Add TA" href={"/course/view_teaching_assistants?id="+(parseInt(this.props.id)/2).toString()+"&model=Course"}>
                  <img src="/assets/tree_view/add-ta-24.png" />
                </a>
                <a title="Create assignment" href={"/assignments/new?parent_id="+(parseInt(this.props.id)/2).toString()}>
                  <img src="/assets/tree_view/add-assignment-24.png" />
                </a>
                <a title="Add participants" href={"/participants/list?id="+(parseInt(this.props.id)/2).toString()+"&model=Course"}>
                  <img src="/assets/tree_view/add-participant-24.png" />
                </a>
                <a title="Create teams" href={"/teams/list?id="+(parseInt(this.props.id)/2).toString()+"&type=Course"}>
                  <img src="/assets/tree_view/create-teams-24.png" />
                </a>
                <a title="View grade summary by student" href={"/assessment360/course_student_grade_summary?course_id="+(parseInt(this.props.id)/2).toString()}>
                  <img src="/assets/tree_view/360-dashboard-24.png" />
                </a>
                <a title="Assign survey" href={"/survey_deployment/new?id="+(parseInt(this.props.id)/2).toString()+"&type=CourseSurveyDeployment"}>
                  <img src="/assets/tree_view/assign-survey-24.png" />
                </a>
                <a title="View aggregated teammate & meta reviews" href={"/assessment360/all_students_all_reviews?course_id="+(parseInt(this.props.id)/2).toString()}>
                  <span style={{"fontSize": "22px", "top": "8px"}} className="glyphicon glyphicon-list-alt"></span>
                </a>
              </span>
            )
          }

With a centralized implementation, we consolidated this information into const node_attribute. (See code repository for implementations for questionnaire and assignments, Course only shown for simplicity of demonstrating implementation).

const node_attributes = {
isCourse(name) {
    return name === 'course' || name === "courses"
  },

course: {
    plural: "course",
    actions: [ {
      title: "Add TA",
      href: "/course/view_teaching_assistants?model=Course&id=",
      src: "/assets/tree_view/add-ta-24.png",
    },
    {
      title: "Create assignment",
      href: "/assignments/new?parent_id=",
      src: "/assets/tree_view/add-assignment-24.png",
    },
    {
      title: "Add participants",
      href: "/participants/list?model=Course&id=",
      src: "/assets/tree_view/add-participant-24.png",
    },
    {
      title: "Create teams",
      href: "/teams/list?type=Course&id=",
      src: "/assets/tree_view/create-teams-24.png",
    },
    {
      title: "View grade summary by student",
      href: "/assessment360/course_student_grade_summary?course_id=",
      src: "/assets/tree_view/360-dashboard-24.png",
    },
    {
      title: "Assign survey",
      href: "/survey_deployment/new?type=CourseSurveyDeployment&id=",
      src: "/assets/tree_view/assign-survey-24.png",
    },
    {
      title: "View aggregated teammate & meta reviews",
      href: "/assessment360/all_students_all_reviews?course_id=",
      src: null,
      extra: <span style={{"fontSize": "22px", "top": "8px"}} className="glyphicon glyphicon-list-alt"></span>
    }],
    getActions: function (id) {
    return (node_attributes.course.actions.map((action) =>
        (action.src
          ? <a title={action.title} href={action.href + id}> <img src={action.src} /></a>
          : <a title={action.title} href={action.href + id}> {action.extra} </a>)))
    }
  },

With this new implementation to obtain these actions, we just use the getActions function. Additionally now call the is{Tab} to receive the name of the tab.

      if (node_attributes.isCourse(this.props.dataType)) {
          moreContent.push(<br/>)
          moreContent.push(...node_attributes.course.getActions(parseInt(this.props.id)/2))
        }

Code improvements

Unused Methods/Functions

Unused Methods and Functions were commented out

• Example 1:

  function showIntelligentAssignmentDialog() {
    jQuery( "#intelligent_assignment_dialog" ).dialog({ closeText: "hide", modal: true, resizable: false, width: 500 });
  }

• Example 2:

The assignments tab did not have any data when we used the reset debugging extension (i.e. the childNodes attribute was null). Thus we assumed there was no data to display underneath each assignment

    def children_node_ng
    flash[:error] = "Invalid JSON in the TreeList" unless json_valid? params[:reactParams][:child_nodes]
    child_nodes = child_nodes_from_params(params[:reactParams][:child_nodes])
    tmp_res = {}
    unless child_nodes.blank?
      child_nodes.each do |node|
        initialize_fnode_update_children(params, node, tmp_res)
      end
    end

References

1. Expertiza on GitHub
2. GitHub Project Repository Fork
3. The live Expertiza website
4. Demo link
5. Expertiza project documentation wiki
6. Rspec Documentation
7. Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin