Commit e51ebfd730a320854f9fd6813035b0847e21ab73
Exists in
master
and in
29 other branches
Merge branch 'newsletter-emails-import' into 'master'
Fixes compatibility in newsletter plugin with CSV exported by Excel Allow different types of delimiter used for the CSV file with additional emails imported by newsletter plugin. This change is meant to improve compatibility for users under Windows in pt br and Excel, which only exports semicolon delimited CSV. See merge request !683
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 | 154 | headers ||= false |
155 | 155 | |
156 | 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 | 164 | end |
161 | - self.additional_recipients = parsed_recipients | |
162 | 165 | else |
163 | 166 | #FIXME find a better way to deal with errors |
164 | 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 | 29 | :person => fast_create(Person)) |
30 | 30 | enabled_newsletters << newsletter.id if enabled |
31 | 31 | end |
32 | - assert_equal enabled_newsletters, NewsletterPlugin::Newsletter.enabled.map(&:id) | |
32 | + assert_equivalent enabled_newsletters, NewsletterPlugin::Newsletter.enabled.map(&:id) | |
33 | 33 | end |
34 | 34 | |
35 | 35 | should 'people of newsletters are the same environment members' do |
... | ... | @@ -214,6 +214,35 @@ EOS |
214 | 214 | assert_equivalent ["Coop1", "Coop2", "Coop3"], newsletter.additional_recipients.map { |recipient| recipient[:name] } |
215 | 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 | 246 | should 'retrieve blogs related to newsletter' do |
218 | 247 | environment = fast_create Environment |
219 | 248 | community = fast_create(Community, :environment_id => environment.id) | ... | ... |