Commit df2897b2afec602fbe3f9c6892589f5ef11a52af
1 parent
014031e1
Exists in
master
and in
29 other branches
newsletter: imported CSV can use semicolon as delimiter
Windows systems with pt_br have problem in exporting CSV with comma as delimiter through Excel, so to improve compatibility I decided to loop through delimiter types when parsing and importing recipients. Since I try different types of delimiter and stick with the first one that gives me a valid content, it's always possible (albeit unlikely) the file can be parsed incorrectly.
Showing
2 changed files
with
37 additions
and
5 deletions
Show diff stats
plugins/newsletter/lib/newsletter_plugin/newsletter.rb
@@ -154,11 +154,14 @@ class NewsletterPlugin::Newsletter < Noosfero::Plugin::ActiveRecord | @@ -154,11 +154,14 @@ class NewsletterPlugin::Newsletter < Noosfero::Plugin::ActiveRecord | ||
154 | headers ||= false | 154 | headers ||= false |
155 | 155 | ||
156 | if File.extname(file.original_filename) == '.csv' | 156 | if File.extname(file.original_filename) == '.csv' |
157 | - parsed_recipients = [] | ||
158 | - CSV.foreach(file.path, headers: headers) do |row| | ||
159 | - parsed_recipients << {name: row[name_column.to_i - 1], email: row[email_column.to_i - 1]} | 157 | + [",", ";", "\t"].each do |sep| |
158 | + parsed_recipients = [] | ||
159 | + CSV.foreach(file.path, { headers: headers, col_sep: sep }) do |row| | ||
160 | + parsed_recipients << {name: row[name_column.to_i - 1], email: row[email_column.to_i - 1]} | ||
161 | + end | ||
162 | + self.additional_recipients = parsed_recipients | ||
163 | + break if self.valid? || !self.errors.include?(:additional_recipients) | ||
160 | end | 164 | end |
161 | - self.additional_recipients = parsed_recipients | ||
162 | else | 165 | else |
163 | #FIXME find a better way to deal with errors | 166 | #FIXME find a better way to deal with errors |
164 | self.errors.add(:additional_recipients, _("have unknown file type: %s" % file.original_filename)) | 167 | self.errors.add(:additional_recipients, _("have unknown file type: %s" % file.original_filename)) |
plugins/newsletter/test/unit/newsletter_plugin_newsletter_test.rb
@@ -29,7 +29,7 @@ class NewsletterPluginNewsletterTest < ActiveSupport::TestCase | @@ -29,7 +29,7 @@ class NewsletterPluginNewsletterTest < ActiveSupport::TestCase | ||
29 | :person => fast_create(Person)) | 29 | :person => fast_create(Person)) |
30 | enabled_newsletters << newsletter.id if enabled | 30 | enabled_newsletters << newsletter.id if enabled |
31 | end | 31 | end |
32 | - assert_equal enabled_newsletters, NewsletterPlugin::Newsletter.enabled.map(&:id) | 32 | + assert_equivalent enabled_newsletters, NewsletterPlugin::Newsletter.enabled.map(&:id) |
33 | end | 33 | end |
34 | 34 | ||
35 | should 'people of newsletters are the same environment members' do | 35 | should 'people of newsletters are the same environment members' do |
@@ -214,6 +214,35 @@ EOS | @@ -214,6 +214,35 @@ EOS | ||
214 | assert_equivalent ["Coop1", "Coop2", "Coop3"], newsletter.additional_recipients.map { |recipient| recipient[:name] } | 214 | assert_equivalent ["Coop1", "Coop2", "Coop3"], newsletter.additional_recipients.map { |recipient| recipient[:name] } |
215 | end | 215 | end |
216 | 216 | ||
217 | + should 'provide flexibility for CSV file when parsing additional recipients' do | ||
218 | + content_semicolon = <<-EOS | ||
219 | +Coop1;name1@example.com | ||
220 | +Coop2;name2@example.com | ||
221 | +Coop3;name3@example.com | ||
222 | +EOS | ||
223 | + | ||
224 | + content_tab = <<-EOS | ||
225 | +Coop1\tname1@example.com | ||
226 | +Coop2\tname2@example.com | ||
227 | +Coop3\tname3@example.com | ||
228 | +EOS | ||
229 | + [content_semicolon, content_tab].each do |content| | ||
230 | + file = Tempfile.new(['recipients', '.csv']) | ||
231 | + file.write(content) | ||
232 | + file.rewind | ||
233 | + | ||
234 | + environment = fast_create Environment | ||
235 | + newsletter = NewsletterPlugin::Newsletter.create!(:environment => environment, :person => fast_create(Person)) | ||
236 | + newsletter.import_recipients(Rack::Test::UploadedFile.new(file, 'text/csv')) | ||
237 | + | ||
238 | + file.close | ||
239 | + file.unlink | ||
240 | + | ||
241 | + assert_equivalent ["name1@example.com", "name2@example.com", "name3@example.com"], newsletter.additional_recipients.map { |recipient| recipient[:email] } | ||
242 | + assert_equivalent ["Coop1", "Coop2", "Coop3"], newsletter.additional_recipients.map { |recipient| recipient[:name] } | ||
243 | + end | ||
244 | + end | ||
245 | + | ||
217 | should 'retrieve blogs related to newsletter' do | 246 | should 'retrieve blogs related to newsletter' do |
218 | environment = fast_create Environment | 247 | environment = fast_create Environment |
219 | community = fast_create(Community, :environment_id => environment.id) | 248 | community = fast_create(Community, :environment_id => environment.id) |