Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/import excel error with nil header cells #113

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def install_module(path)

# Temporary fix to overcome the issue with sass-embedded, see:
# https://github.com/decidim/decidim/pull/11074
system("npm i sass-embedded@~1.62.0")
# system("npm i sass-embedded@~1.62.0")
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ $(() => {
modifyResult: (item, valueItem) => {
const sanitizedSearch = currentSearch.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&");
const re = new RegExp(`(${sanitizedSearch})`, "gi");
const replacedText = item.textContent.replace(re, '<strong class="search-highlight">$1</strong>');
const replacedText = valueItem.label.replace(re, '<strong class="search-highlight">$1</strong>');
item.innerHTML = replacedText;
item.dataset.value = valueItem.value;
},
Expand Down
11 changes: 7 additions & 4 deletions lib/decidim/term_customizer/import/importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,15 @@ def collection_data
data_headers = []
reader.new(file).read_rows do |rowdata, index|
if index.zero?
data_headers = rowdata.map(&:to_sym)
data_headers = rowdata.compact_blank.map(&:to_sym)
else
next if rowdata.blank?

@collection_data <<
rowdata.each_with_index.to_h do |val, ind|
[data_headers[ind], val]
end
data_headers.each_with_index.map do |header, ind|
val = rowdata[ind]
[header, val] if header.present? && val.present?
end.compact.to_h
end
end

Expand Down
6 changes: 5 additions & 1 deletion lib/decidim/term_customizer/import/readers/xlsx.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
workbook = RubyXL::Parser.parse(file)
sheet = workbook.worksheets[0]
sheet.each_with_index do |row, index|
yield row.cells.map(&:value), index
if row
yield row.cells.map { |c| c && c.value }, index
else
yield [], index

Check warning on line 21 in lib/decidim/term_customizer/import/readers/xlsx.rb

View check run for this annotation

Codecov / codecov/patch

lib/decidim/term_customizer/import/readers/xlsx.rb#L21

Added line #L21 was not covered by tests
end
end
end
end
Expand Down
25 changes: 25 additions & 0 deletions lib/decidim/term_customizer/test/rspec_support/capybara.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

require "selenium-webdriver"

module Decidim
# This is being added because of the issues with the chrome-driver
# in version 120 or later, and this can be removed after this pr#12160
# being merged(more info https://github.com/decidim/decidim/pull/12159).
Capybara.register_driver :headless_chrome do |app|
options = ::Selenium::WebDriver::Chrome::Options.new
options.args << "--headless=new"
options.args << "--no-sandbox"
options.args << if ENV["BIG_SCREEN_SIZE"].present?
"--window-size=1920,3000"
else
"--window-size=1920,1080"
end
options.args << "--ignore-certificate-errors" if ENV["TEST_SSL"]
Capybara::Selenium::Driver.new(
app,
browser: :chrome,
capabilities: [options]
)
end
end
Binary file not shown.
6 changes: 6 additions & 0 deletions spec/lib/decidim/term_customizer/import/readers/xlsx_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@
let(:file) { file_fixture("set-translations.xlsx") }

it_behaves_like "translation import reader"

context "with xlsx files containing nils in header" do
let(:file) { file_fixture("set-translations-with-nils.xlsx") }

it_behaves_like "translation import reader"
end
end
2 changes: 1 addition & 1 deletion spec/shared/translation_importer_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
it "yields all the data correctly" do
data = []
subject.read_rows do |row, index|
data[index] = row.map(&:to_s)
data[index] = row.compact_blank.map(&:to_s)
end

expected_array = []
Expand Down
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
File.expand_path(File.join(__dir__, "decidim_dummy_app"))

require "decidim/dev/test/base_spec_helper"
# This is being added because of the issues with the chrome-driver
# in version 120 or later, and this can be removed after this pr#12160
# being merged(more info https://github.com/decidim/decidim/pull/12159).
require "#{Dir.pwd}/lib/decidim/term_customizer/test/rspec_support/capybara.rb"

RSpec.configure do |config|
# Add extra traslation load path for the tests
Expand Down
49 changes: 49 additions & 0 deletions spec/system/expolre_adding_translation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

require "spec_helper"

describe "explore adding translation", type: :system do
let(:organization) { create(:organization) }
let(:user) { create(:user, :admin, :confirmed, organization: organization) }
let!(:translation_set) { create(:translation_set, organization: organization, name: { en: "Dummy set" }) }

context "when the translation value is the list" do
# let!(:translation1) { create(:translation, key: "dummy.translation.one", value: "<ul><li>dummy translation list</li></ul>") }
# let!(:translation2) { create(:translation, key: "dummy.translation.two", value: "dummy translation string") }
let(:translation_hash) do
{
"dummy.translation.one" => "<ul><li>dummy translation list</li></ul>",
"dummy.translation.two" => "dummy translation string"
}
end

before do
switch_to_host(organization.host)
sign_in user
visit decidim_admin_term_customizer.translation_sets_path
click_link "Dummy set"
click_link "Add multiple", match: :first
end

it "Adds the lists properly" do
# rubocop:disable RSpec/AnyInstance
allow_any_instance_of(Decidim::TermCustomizer::TranslationDirectory).to receive(:translations_search).with("dummy tran").and_return(translation_hash)
# rubocop:enable RSpec/AnyInstance

fill_in "Search", with: "dummy tran"
li_element = page.find("li", text: "<ul><li>dummy translation list</li></ul>")
within "ul#autoComplete_list_1" do
expect(li_element).not_to have_css(".hide")
expect(page).to have_css("li", text: "<ul><li>dummy translation list</li></ul>")
expect(page).to have_css("li", text: "dummy translation string")
li_element.click
expect(li_element).not_to have_css("li", text: "<ul><li>dummy translation list</li></ul>")
end
within "table.table-list" do
expect(page).to have_css("th", text: "Translation key")
expect(page).to have_css("td", text: "dummy.translation.one")
expect(page).to have_css("td", text: "<ul><li>dummy translation list</li></ul>")
end
end
end
end