Skip to content

Instantly share code, notes, and snippets.

@abhchand
Created June 25, 2018 20:28
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save abhchand/29742d359bd7cd3465eaf6a0b4f9fcd1 to your computer and use it in GitHub Desktop.
Save abhchand/29742d359bd7cd3465eaf6a0b4f9fcd1 to your computer and use it in GitHub Desktop.
GB-1509 Code Review Changes
diff --git a/app/assets/stylesheets/app/admin/users/imports/index.scss b/app/assets/stylesheets/app/admin/users/imports/index.scss
index 5c2fb2d..81a151e 100644
--- a/app/assets/stylesheets/app/admin/users/imports/index.scss
+++ b/app/assets/stylesheets/app/admin/users/imports/index.scss
@@ -27,8 +27,6 @@
}
.admin-users-imports-index__datatable-row {
- @extend .link_danger;
-
border: 1px solid $light_gray;
&:nth-child(even) {
background: $white;
@@ -36,6 +34,7 @@
.admin-users-imports-index__datatable-cell--errors {
a {
+ @extend .link_danger;
color: $cranberry;
}
diff --git a/app/controllers/admin/users/imports_controller.rb b/app/controllers/admin/users/imports_controller.rb
index 1bfacee..adf361b 100644
--- a/app/controllers/admin/users/imports_controller.rb
+++ b/app/controllers/admin/users/imports_controller.rb
@@ -10,8 +10,7 @@ class Admin::Users::ImportsController < ApplicationController
layout "admin"
def index
- @import = Import.new
- @imports = Import.where(import_type: "employee").order(id: :desc)
+ @imports = find_all_imports
end
def create
@@ -22,6 +21,7 @@ class Admin::Users::ImportsController < ApplicationController
queue_import_job
redirect_to admin_users_imports_path
else
+ @imports = find_all_imports
flash.now[:error] = @import.errors.full_messages.to_sentence
render :index
end
@@ -40,6 +40,10 @@ class Admin::Users::ImportsController < ApplicationController
params.require(:import).permit(:source_file)
end
+ def find_all_imports
+ Import.where(import_type: "employee").order(id: :desc)
+ end
+
def queue_import_job
ImportJob.perform_async(@import.id, current_user.id)
end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 89ea4ba..048c180 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -144,6 +144,9 @@ module ApplicationHelper
path["style"] = options[:path][:style] if options[:path][:style].present?
end
+ title = doc.at_css "title"
+ title.inner_html = options[:title] if options[:title].present?
+
apply_presentation_info(svg, doc) if options[:presentation_only]
raw doc
diff --git a/app/views/admin/users/imports/index.html.erb b/app/views/admin/users/imports/index.html.erb
index 0d48249..00e658b 100644
--- a/app/views/admin/users/imports/index.html.erb
+++ b/app/views/admin/users/imports/index.html.erb
@@ -69,7 +69,7 @@
<% if import.error_file_file_name %>
<%= link_to(t(".error_file_label", count: import.error_row_count), import.error_file.url) %>
<% else %>
- <%= embedded_svg("icons/check.svg", class: "svg_icon24 svg_teal") %>
+ <%= embedded_svg("icons/check.svg", class: "svg_icon24 svg_teal", title: t(".import_success")) %>
<% end %>
</td>
</tr>
diff --git a/app/views/community_post/events/_event_rsvp.html.erb b/app/views/community_post/events/_event_rsvp.html.erb
index 9f330d4..918e21a 100644
--- a/app/views/community_post/events/_event_rsvp.html.erb
+++ b/app/views/community_post/events/_event_rsvp.html.erb
@@ -2,12 +2,12 @@
<% case %>
<% when post.responded_as_attending?(current_user) %>
<div class="community-post-card__post-event-rsvp-yes-confirmation">
- <%= embedded_svg("icons/check.svg", class: "svg_white svg_icon12 community-post-card__post-event-rsvp-icon") %> <%= t(".yes_confirmation") %>
+ <%= embedded_svg("icons/check.svg", presentation_only: true, class: "svg_white svg_icon12 community-post-card__post-event-rsvp-icon") %> <%= t(".yes_confirmation") %>
</div>
<div class="community-post-card__post-event-rsvp-yes-change-to-no"><%= t(".yes_change_to_no") %></div>
<% when post.responded_as_not_attending?(current_user) %>
<div class="community-post-card__post-event-rsvp-no-confirmation">
- <%= embedded_svg("icons/check.svg", class: "svg_white svg_icon12 community-post-card__post-event-rsvp-icon") %> <%= t(".no_confirmation") %>
+ <%= embedded_svg("icons/check.svg", presentation_only: true, class: "svg_white svg_icon12 community-post-card__post-event-rsvp-icon") %> <%= t(".no_confirmation") %>
</div>
<div class="community-post-card__post-event-rsvp-no-change-to-yes"><%= t(".no_change_to_yes") %></div>
<% when !post.community.has_member?(current_user) %>
diff --git a/config/locales/views/admin/imports/index.yml b/config/locales/views/admin/imports/index.yml
index 1937738..6725175 100644
--- a/config/locales/views/admin/imports/index.yml
+++ b/config/locales/views/admin/imports/index.yml
@@ -11,6 +11,7 @@ en:
error_file_label:
one: 1 error
other: "%{count} errors"
+ import_success: File was imported successfully.
in_progress: Import In Progress
columns:
id: ID
diff --git a/spec/controllers/admin/users/imports_controller_spec.rb b/spec/controllers/admin/users/imports_controller_spec.rb
index 1b95d11..ad9fbb6 100644
--- a/spec/controllers/admin/users/imports_controller_spec.rb
+++ b/spec/controllers/admin/users/imports_controller_spec.rb
@@ -20,7 +20,6 @@ RSpec.describe Admin::Users::ImportsController do
get :index
- expect(assigns[:import]).to be_an_instance_of(Import)
expect(assigns[:imports]).to eq([import_three, import_one])
expect(response).to render_template("admin/users/imports/index")
diff --git a/spec/features/admin/users/imports/importing_file_spec.rb b/spec/features/admin/users/imports/importing_file_spec.rb
index 85c94a2..08eca61 100644
--- a/spec/features/admin/users/imports/importing_file_spec.rb
+++ b/spec/features/admin/users/imports/importing_file_spec.rb
@@ -67,6 +67,31 @@ feature "Importing File" do
end
end
+ context "file is invalid" do
+ it "sets the flash message and renders the index page" do
+ sign_in(user)
+ visit admin_users_imports_path
+
+ @import = build(:import, :with_employee_file)
+
+ expect(Import).to receive(:new) { @import }
+ expect(@import).to(
+ receive(:import_type=).and_wrap_original do |m|
+ m.call("foo")
+
+ # Un-stub the initial stub on Import.new becaue it gets called
+ # again while rendering the view
+ expect(Import).to receive(:new).and_call_original
+ end
+ )
+
+ expect { upload_file }.to_not(change { Import.count })
+
+ expect(page).to have_current_path(admin_users_imports_path)
+ expect(page).to have_content("Import type is not included in the list")
+ end
+ end
+
def upload_file
attach_file
submit
diff --git a/spec/views/admin/users/imports/index.html.erb_spec.rb b/spec/views/admin/users/imports/index.html.erb_spec.rb
index b409387..2cf8fa6 100644
--- a/spec/views/admin/users/imports/index.html.erb_spec.rb
+++ b/spec/views/admin/users/imports/index.html.erb_spec.rb
@@ -88,7 +88,8 @@ RSpec.describe "admin/users/imports/index.html.erb", type: :view do
.to have_content(administrator.to_local_time(import.completed_at))
# Displays `icons/check.svg`
- expect(import_row.find(field_css_for("errors"))).to have_content("check")
+ expect(import_row.find(field_css_for("errors")))
+ .to have_content(t("#{@t_prefix}.import_success"))
end
context "source_row_count is not present yet" do
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment