From 512159a9c2977866a597f5915036c81cafea18f2 Mon Sep 17 00:00:00 2001 From: "behrooz.torki@digitalnatives.hu" Date: Thu, 8 Jun 2017 13:48:45 +0200 Subject: [PATCH 1/2] Removes status from course --- .../20170608114513_remove_course_status.exs | 9 +++ test/controllers/course_controller_test.exs | 73 ++----------------- test/models/course_test.exs | 62 +--------------- test/models/offered_course_test.exs | 3 +- test/support/factory.ex | 3 +- web/controllers/course_controller.ex | 26 ++++--- web/models/course/course.ex | 11 +-- web/models/course/course_helper.ex | 27 ++----- web/templates/course/form.html.eex | 5 -- web/templates/course/index.html.eex | 2 - web/templates/course/show.html.eex | 5 -- web/views/offered_course_view.ex | 5 +- 12 files changed, 50 insertions(+), 181 deletions(-) create mode 100644 priv/repo/migrations/20170608114513_remove_course_status.exs diff --git a/priv/repo/migrations/20170608114513_remove_course_status.exs b/priv/repo/migrations/20170608114513_remove_course_status.exs new file mode 100644 index 00000000..6a42be4b --- /dev/null +++ b/priv/repo/migrations/20170608114513_remove_course_status.exs @@ -0,0 +1,9 @@ +defmodule CoursePlanner.Repo.Migrations.RemoveCourseStatus do + use Ecto.Migration + + def change do + alter table(:courses) do + remove :status + end + end +end diff --git a/test/controllers/course_controller_test.exs b/test/controllers/course_controller_test.exs index e87469a1..6aacd3e7 100644 --- a/test/controllers/course_controller_test.exs +++ b/test/controllers/course_controller_test.exs @@ -5,7 +5,7 @@ defmodule CoursePlanner.CourseControllerTest do import CoursePlanner.Factory - @valid_attrs %{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %{hour: 14, min: 0, sec: 0}, status: "Planned", syllabus: "some content"} + @valid_attrs %{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %{hour: 14, min: 0, sec: 0}, syllabus: "some content"} @invalid_attrs %{} @user %User{ name: "Test User", @@ -78,43 +78,18 @@ defmodule CoursePlanner.CourseControllerTest do assert html_response(conn, 200) =~ "Edit course" end - test "deletes chosen resource when the states is Planned", %{conn: conn} do - course = Repo.insert! %Course{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %Ecto.Time{hour: 2, min: 0, sec: 0}, status: "Planned", syllabus: "some content"} + test "deletes chosen resource", %{conn: conn} do + course = Repo.insert! %Course{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %Ecto.Time{hour: 2, min: 0, sec: 0}, syllabus: "some content"} conn = delete conn, course_path(conn, :delete, course) assert redirected_to(conn) == course_path(conn, :index) refute Repo.get(Course, course.id) end - test "deletes chosen resource when the states is Active", %{conn: conn} do - course = Repo.insert! %Course{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %Ecto.Time{hour: 2, min: 0, sec: 0}, status: "Active", syllabus: "some content"} - conn = delete conn, course_path(conn, :delete, course) - assert redirected_to(conn) == course_path(conn, :index) - soft_deleted_course = Repo.get(Course, course.id) - assert soft_deleted_course.deleted_at - end - - test "deletes chosen resource when the states is Finished", %{conn: conn} do - course = Repo.insert! %Course{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %Ecto.Time{hour: 2, min: 0, sec: 0}, status: "Finished", syllabus: "some content"} - conn = delete conn, course_path(conn, :delete, course) - assert redirected_to(conn) == course_path(conn, :index) - soft_deleted_course = Repo.get(Course, course.id) - assert soft_deleted_course.deleted_at - end - - test "deletes chosen resource when the states is Graduated", %{conn: conn} do - course = Repo.insert! %Course{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %Ecto.Time{hour: 2, min: 0, sec: 0}, status: "Graduated", syllabus: "some content"} - conn = delete conn, course_path(conn, :delete, course) - assert redirected_to(conn) == course_path(conn, :index) - soft_deleted_course = Repo.get(Course, course.id) - assert soft_deleted_course.deleted_at - end - - test "deletes chosen resource when the states is Frozen", %{conn: conn} do - course = Repo.insert! %Course{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %Ecto.Time{hour: 2, min: 0, sec: 0}, status: "Frozen", syllabus: "some content"} - conn = delete conn, course_path(conn, :delete, course) + test "deletes with an invalid id", %{conn: conn} do + conn = delete conn, course_path(conn, :delete, -1) assert redirected_to(conn) == course_path(conn, :index) - soft_deleted_course = Repo.get(Course, course.id) - assert soft_deleted_course.deleted_at + conn = get conn, course_path(conn, :index) + assert html_response(conn, 200) =~ "Course was not found" end test "does not create resource and renders errors when data number_of_sessions is negative", %{conn: conn} do @@ -132,40 +107,6 @@ defmodule CoursePlanner.CourseControllerTest do assert html_response(conn, 200) =~ "New course" end - test "does not create resource and renders errors when data value of status is not valid", %{conn: conn} do - conn = post conn, course_path(conn, :create), course: %{@valid_attrs | status: "random"} - assert html_response(conn, 200) =~ "New course" - end - - test "creates resource and redirects when data is valid and status is Planned", %{conn: conn} do - new_attrs = %{@valid_attrs | status: "Planned"} - conn = post conn, course_path(conn, :create), course: new_attrs - assert redirected_to(conn) == course_path(conn, :index) - assert Repo.get_by(Course, new_attrs) - end - - test "creates resource and redirects when data is valid and status is Active", %{conn: conn} do - new_attrs = %{@valid_attrs | status: "Active"} - conn = post conn, course_path(conn, :create), course: new_attrs - assert redirected_to(conn) == course_path(conn, :index) - assert Repo.get_by(Course, new_attrs) - end - - test "does not create resource and renders errors when data value of status is Finished", %{conn: conn} do - conn = post conn, course_path(conn, :create), course: %{@valid_attrs | status: "Finished"} - assert html_response(conn, 200) =~ "New course" - end - - test "does not create resource and renders errors when data value of status is Graduated", %{conn: conn} do - conn = post conn, course_path(conn, :create), course: %{@valid_attrs | status: "Graduated"} - assert html_response(conn, 200) =~ "New course" - end - - test "does not create resource and renders errors when data value of status is Frozen", %{conn: conn} do - conn = post conn, course_path(conn, :create), course: %{@valid_attrs | status: "Frozen"} - assert html_response(conn, 200) =~ "New course" - end - test "does not shows chosen resource for non coordinator user", %{conn: _conn} do student_conn = login_as(:student) teacher_conn = login_as(:teacher) diff --git a/test/models/course_test.exs b/test/models/course_test.exs index 1a1f9fb3..1bf3264c 100644 --- a/test/models/course_test.exs +++ b/test/models/course_test.exs @@ -3,7 +3,7 @@ defmodule CoursePlanner.CourseTest do alias CoursePlanner.Course, as: Course - @valid_attrs %{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %{hour: 14, min: 0, sec: 0}, status: "Planned", syllabus: "some content"} + @valid_attrs %{description: "some content", name: "some content", number_of_sessions: 42, session_duration: %{hour: 14, min: 0, sec: 0}, syllabus: "some content"} @invalid_attrs %{} test "changeset with valid attributes" do @@ -30,64 +30,4 @@ defmodule CoursePlanner.CourseTest do changeset = Course.changeset(%Course{}, %{ @valid_attrs | number_of_sessions: 100_000_000 }) refute changeset.valid? end - - test "changeset with invalid status" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "random" }) - refute changeset.valid? - end - - test "changeset with status Planned" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Planned" }) - assert changeset.valid? - end - - test "changeset with status Active" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Active" }) - assert changeset.valid? - end - - test "changeset with status Finished" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Finished" }) - assert changeset.valid? - end - - test "changeset with status Graduated" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Graduated" }) - assert changeset.valid? - end - - test "changeset with status Frozen" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Frozen" }) - assert changeset.valid? - end - - test "changeset with invalid status with :create" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "random" }, :create) - refute changeset.valid? - end - - test "changeset with status Planned with :create" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Planned" }, :create) - assert changeset.valid? - end - - test "changeset with status Active with :create" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Active" }, :create) - assert changeset.valid? - end - - test "changeset with status Finished with :create" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Finished" }, :create) - refute changeset.valid? - end - - test "changeset with status Graduated with :create" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Graduated" }, :create) - refute changeset.valid? - end - - test "changeset with status Frozen with :create" do - changeset = Course.changeset(%Course{}, %{ @valid_attrs | status: "Frozen" }, :create) - refute changeset.valid? - end end diff --git a/test/models/offered_course_test.exs b/test/models/offered_course_test.exs index b8e898cb..de62eabc 100644 --- a/test/models/offered_course_test.exs +++ b/test/models/offered_course_test.exs @@ -55,8 +55,7 @@ defmodule CoursePlanner.OfferedCourseTest do name: "Course", description: "Description", number_of_sessions: 1, - session_duration: Ecto.Time.cast!("01:00:00"), - status: "Active" + session_duration: Ecto.Time.cast!("01:00:00") }) end end diff --git a/test/support/factory.ex b/test/support/factory.ex index 82b33bbc..2a85f367 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -65,8 +65,7 @@ alias CoursePlanner.{User, Course, OfferedCourse, Class, Attendance} name: sequence(:name, &"course-#{&1}"), description: "Description", number_of_sessions: 1, - session_duration: "01:00:00", - status: "Planned" + session_duration: "01:00:00" } end diff --git a/web/controllers/course_controller.ex b/web/controllers/course_controller.ex index feabf620..0cafd528 100644 --- a/web/controllers/course_controller.ex +++ b/web/controllers/course_controller.ex @@ -1,14 +1,13 @@ defmodule CoursePlanner.CourseController do use CoursePlanner.Web, :controller - alias CoursePlanner.Course - alias CoursePlanner.CourseHelper + alias CoursePlanner.{Repo, Course, CourseHelper} import Canary.Plugs plug :authorize_resource, model: Course def index(conn, _params) do - courses = CourseHelper.all_none_deleted() + courses = Repo.all(Course) render(conn, "index.html", courses: courses) end @@ -66,11 +65,20 @@ defmodule CoursePlanner.CourseController do end def delete(%{assigns: %{current_user: current_user}} = conn, %{"id" => id}) do - course = Repo.get!(Course, id) - CourseHelper.delete(course) - CourseHelper.notify_user_course(course, current_user, :course_deleted) - conn - |> put_flash(:info, "Course deleted successfully.") - |> redirect(to: course_path(conn, :index)) + case CourseHelper.delete(id) do + {:ok, course} -> + CourseHelper.notify_user_course(course, current_user, :course_deleted) + conn + |> put_flash(:info, "Course deleted successfully.") + |> redirect(to: course_path(conn, :index)) + {:error, :not_found} -> + conn + |> put_flash(:error, "Course was not found.") + |> redirect(to: course_path(conn, :index)) + {:error, _changeset} -> + conn + |> put_flash(:error, "Something went wrong.") + |> redirect(to: course_path(conn, :index)) + end end end diff --git a/web/models/course/course.ex b/web/models/course/course.ex index fd034d0d..a4c56862 100644 --- a/web/models/course/course.ex +++ b/web/models/course/course.ex @@ -4,7 +4,7 @@ defmodule CoursePlanner.Course do """ use CoursePlanner.Web, :model - alias CoursePlanner.{OfferedCourse, Types} + alias CoursePlanner.OfferedCourse schema "courses" do field :name, :string @@ -12,10 +12,8 @@ defmodule CoursePlanner.Course do field :number_of_sessions, :integer field :session_duration, Ecto.Time field :syllabus, :string - field :status, Types.EntityStatus - field :deleted_at, :naive_datetime - has_many :offered_courses, OfferedCourse, on_replace: :delete + has_many :offered_courses, OfferedCourse, on_replace: :delete, on_delete: :delete_all has_many :terms, through: [:offered_courses, :term] timestamps() @@ -31,18 +29,17 @@ defmodule CoursePlanner.Course do :description, :number_of_sessions, :session_duration, - :syllabus, :status, :deleted_at + :syllabus ] struct |> cast(params, target_params) - |> validate_required([:name, :description, :number_of_sessions, :session_duration, :status]) + |> validate_required([:name, :description, :number_of_sessions, :session_duration]) |> validate_number(:number_of_sessions, greater_than: 0, less_than: 100_000_000) end def changeset(struct, params, :create) do struct |> changeset(params) - |> validate_inclusion(:status, ["Planned", "Active"]) end end diff --git a/web/models/course/course_helper.ex b/web/models/course/course_helper.ex index 11a80a7e..6a8da3a1 100644 --- a/web/models/course/course_helper.ex +++ b/web/models/course/course_helper.ex @@ -3,31 +3,18 @@ defmodule CoursePlanner.CourseHelper do This module provides custom functionality for controller over the model """ use CoursePlanner.Web, :model - import Ecto.DateTime, only: [utc: 0] - alias CoursePlanner.{Repo, Course, Notifier, Coordinators, Notifier.Notification} + alias CoursePlanner.{Repo, Course, Coordinators, Notifier, Notifier.Notification} - def delete(course) do - case course.status do - "Planned" -> hard_delete_course(course) - _ -> soft_delete_course(course) + def delete(id) do + course = Repo.get(Course, id) + if is_nil(course) do + {:error, :not_found} + else + Repo.delete(course) end end - defp soft_delete_course(course) do - changeset = change(course, %{deleted_at: utc()}) - Repo.update(changeset) - end - - defp hard_delete_course(course) do - Repo.delete!(course) - end - - def all_none_deleted do - query = from c in Course , where: is_nil(c.deleted_at) - Repo.all(query) - end - def notify_user_course(course, current_user, notification_type, path \\ "/") do course |> get_subscribed_users() diff --git a/web/templates/course/form.html.eex b/web/templates/course/form.html.eex index 7185589f..7ff82836 100644 --- a/web/templates/course/form.html.eex +++ b/web/templates/course/form.html.eex @@ -35,11 +35,6 @@ <%= error_tag f, :syllabus %> -
- <%= select(f, :status, ["Planned", "Active"], prompt: "Choose the status", selected: "Planned") %> - <%= error_tag f, :status %> -
-
<%= submit "Submit", class: "btn btn-primary" %>
diff --git a/web/templates/course/index.html.eex b/web/templates/course/index.html.eex index 5416bd48..5bd33ffd 100644 --- a/web/templates/course/index.html.eex +++ b/web/templates/course/index.html.eex @@ -6,7 +6,6 @@ Name Number of sessions Session duration - Status @@ -17,7 +16,6 @@ <%= course.name %> <%= course.number_of_sessions %> <%= course.session_duration %> - <%= course.status %> <%= link "Show", to: course_path(@conn, :show, course), class: "btn btn-default btn-xs" %> diff --git a/web/templates/course/show.html.eex b/web/templates/course/show.html.eex index 1980ab0a..b0544678 100644 --- a/web/templates/course/show.html.eex +++ b/web/templates/course/show.html.eex @@ -27,11 +27,6 @@ <%= @course.syllabus %> -
  • - Status: - <%= @course.status %> -
    • - <%= link "Edit", to: course_path(@conn, :edit, @course) %> diff --git a/web/views/offered_course_view.ex b/web/views/offered_course_view.ex index e0a903b0..1e9e2a7d 100644 --- a/web/views/offered_course_view.ex +++ b/web/views/offered_course_view.ex @@ -1,7 +1,7 @@ defmodule CoursePlanner.OfferedCourseView do use CoursePlanner.Web, :view - alias CoursePlanner.{CourseHelper, Teachers, Terms, Students} + alias CoursePlanner.{Repo, Teachers, Terms, Students, Course} alias Ecto.Changeset def terms_to_select do @@ -14,7 +14,8 @@ defmodule CoursePlanner.OfferedCourseView do end def courses_to_select do - CourseHelper.all_none_deleted() + Course + |> Repo.all() |> Enum.map(&{&1.name, &1.id}) end From 45c86a537d2312f8cb4042ac2a9419d65f08dc66 Mon Sep 17 00:00:00 2001 From: "behrooz.torki@digitalnatives.hu" Date: Thu, 8 Jun 2017 13:56:21 +0200 Subject: [PATCH 2/2] removes the deleted_at field too --- priv/repo/migrations/20170608114513_remove_course_status.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/priv/repo/migrations/20170608114513_remove_course_status.exs b/priv/repo/migrations/20170608114513_remove_course_status.exs index 6a42be4b..8f67a2f4 100644 --- a/priv/repo/migrations/20170608114513_remove_course_status.exs +++ b/priv/repo/migrations/20170608114513_remove_course_status.exs @@ -4,6 +4,7 @@ defmodule CoursePlanner.Repo.Migrations.RemoveCourseStatus do def change do alter table(:courses) do remove :status + remove :deleted_at end end end