Can create new courses

dashboard/app/controllers/courses_controller.rb

@@ -1,4 +1,6 @@
 class CoursesController < ApplicationController
+  before_action :require_levelbuilder_mode, except: [:index, :show]
+
   def index
     @recent_courses = current_user.try(:recent_courses)
     @is_teacher = !!(current_user && current_user.teacher?)
@@ -8,9 +10,16 @@ def index
   end
 
   def show
-    course_name = params[:course_name].tr('-', '_').titleize
-    course = Course.find_by_name(course_name)
-    raise ActiveRecord::RecordNotFound unless course
+    course = Course.find_by_name(params[:course_name])
+    unless course
+      # PLC courses have different ways of getting to name. ideally this goes
+      # away eventually
+      course_name = params[:course_name].tr('-', '_').titleize
+      course = Course.find_by_name!(course_name)
+      # only support this alternative course name for plc courses
+      raise ActiveRecord::RecordNotFound unless course.plc_course
+    end
+
     if course.plc_course
       authorize! :show, Plc::UserCourseEnrollment
       user_course_enrollments = [Plc::UserCourseEnrollment.find_by(user: current_user, plc_course: course.plc_course)]
@@ -20,4 +29,21 @@ def show
 
     render 'show', locals: {course: course}
   end
+
+  def new
+    render 'new'
+  end
+
+  def create
+    course = Course.new(name: params.require(:course).require(:name))
+    if course.save
+      redirect_to action: :edit, course_name: course.name
+    else
+      render 'new', locals: {error_messages: course.errors.full_messages}
+    end
+  end
+
+  def edit
+    render text: 'Not yet implemented'
+  end
 end

dashboard/app/models/course.rb

@@ -18,6 +18,32 @@ class Course < ApplicationRecord
   has_one :plc_course, class_name: 'Plc::Course'
   has_many :course_scripts, -> {order('position ASC')}
 
+  validates :name,
+    presence: true,
+    uniqueness: {case_sensitive: false}
+
+  class CourseNameValidator < ActiveModel::Validator
+    def validate(course)
+      return if course.plc_course
+
+      unless /\A[a-z0-9\-]+\z/ =~ course.name
+        course.errors[:base] << 'can only contain lowercase letters, numbers and dashes'
+      end
+    end
+  end
+
+  validates_with CourseNameValidator
+
+  # As we read and write to files with the course name, to prevent directory
+  # traversal (for security reasons), we do not allow the name to start with a
+  # tilde or dot or contain a slash.
+  validates :name,
+    presence: true,
+    format: {
+      without: /\A~|\A\.|\//,
+      message: 'cannot start with a tilde or dot or contain slashes'
+    }
+
   def summarize
     {
       name: name,

dashboard/app/models/script.rb

@@ -63,8 +63,11 @@ class Script < ActiveRecord::Base
 
   def generate_plc_objects
     if professional_learning_course?
-      course = Course.find_or_create_by!(name: professional_learning_course) do |new_course|
-        Plc::Course.create!(course: new_course)
+      course = Course.find_by_name(professional_learning_course)
+      unless course
+        course = Course.new(name: professional_learning_course)
+        course.plc_course = Plc::Course.create!(course: course)
+        course.save!
       end
       unit = Plc::CourseUnit.find_or_initialize_by(script_id: id)
       unit.update!(

dashboard/app/views/courses/new.html.haml

@@ -0,0 +1,17 @@
+- error_messages = local_assigns[:error_messages]
+
+- if error_messages.try(:any?)
+  #error_explanation
+    %h2
+      = pluralize(error_messages.count, 'error')
+      prohibited this course from being saved:
+    %ul
+      - error_messages.each do |msg|
+        %li= msg
+
+%h1= t('crud.new_model', model: Course.model_name.human)
+= form_for(:course, {url: courses_path}) do |f|
+  .field
+    = f.label :name
+    = f.text_field :name
+  %button.btn.btn-primary{type: 'submit', style: 'margin: 0'} Save Changes

dashboard/test/controllers/courses_controller_test.rb

@@ -5,33 +5,35 @@ class CoursesControllerTest < ActionController::TestCase
 
   setup do
     @teacher = create :teacher
-    plc_course = create :plc_course, name: "My PLC"
+    sign_in @teacher
+
+    plc_course = create :plc_course, name: 'My PLC'
     @course_plc = plc_course.course
-    @course_regular = create :course, name: 'non plc course'
+    @course_regular = create :course, name: 'non-plc-course'
   end
 
   test "plc courses get sent to user_course_enrollments_controller" do
-    sign_in @teacher
-
     get :show, params: {course_name: @course_plc.name}
     assert_template 'plc/user_course_enrollments/index'
+  end
 
-    get :show, params: {course_name: 'my-plc'}
+  test "plc course names get titleized" do
+    get :show, params: {course_name: 'my_plc'}
     assert_template 'plc/user_course_enrollments/index'
   end
 
-  test "regular courses get sent to index" do
-    sign_in @teacher
-
+  test "regular courses get sent to show" do
     get :show, params: {course_name: @course_regular.name}
     assert_template 'courses/show'
+  end
 
-    get :show, params: {course_name: 'non-plc-course'}
-    assert_template 'courses/show'
+  test "regular courses do not get titlized" do
+    assert_raises ActiveRecord::RecordNotFound do
+      get :show, params: {course_name: 'non_plc_course'}
+    end
   end
 
-  test "non exist course throws" do
-    sign_in @teacher
+  test "non existant course throws" do
     assert_raises ActiveRecord::RecordNotFound do
       get :show, params: {course_name: 'nosuchcourse'}
     end

dashboard/test/factories.rb

@@ -529,9 +529,11 @@
   end
   factory :plc_course, class: 'Plc::Course' do
     transient do
-      name 'MyString'
+      name 'plccourse'
+    end
+    after(:build) do |plc_course, evaluator|
+      create(:course, name: evaluator.name, plc_course: plc_course)
     end
-    course {create(:course, name: name)}
   end
 
   factory :level_group, class: LevelGroup do

dashboard/test/models/course_test.rb

@@ -1,7 +1,25 @@
 require 'test_helper'
 
 class CourseTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test "should allow valid course names" do
+    create(:course, name: 'valid-name')
+  end
+
+  test "should not allow uppercase letters in course name" do
+    assert_raises ActiveRecord::RecordInvalid do
+      create(:course, name: 'UpperCase')
+    end
+  end
+
+  test "should not allow spaces in course name" do
+    assert_raises ActiveRecord::RecordInvalid do
+      create(:course, name: 'spaced out')
+    end
+  end
+
+  test "should allow uppercase letters if it is a plc course" do
+    course = Course.new(name: 'PLC Course')
+    course.plc_course = Plc::Course.new(course: course)
+    course.save!
+  end
 end