https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292020-04-07T16:02:37ZRedmineRedmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=971962020-04-07T16:02:37ZTakenori TAKAKItakenory@gmail.com
<ul><li><strong>File</strong> <a href="/attachments/25164">csv_user_import.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/25164/csv_user_import.patch">csv_user_import.patch</a> added</li></ul>I made a patch to achive this feature in the same way as Importing Issue, Importing TimeEntry.<br />Admin users will be able to import the following information from CSV.
<ul>
<li>Login</li>
<li>Firstname, Lastname</li>
<li>Email</li>
<li>Language (e.g. 'en')</li>
<li>Administrator (Yes / No)</li>
<li>Authentication mode</li>
<li>Password</li>
<li>Must change password at next logon (Yes / No)</li>
<li>Status (active / registered / locked)</li>
<li>Custom Fields</li>
</ul> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=975702020-04-28T15:12:10ZGo MAEDA
<ul><li><strong>Target version</strong> set to <i>Candidate for next major release</i></li></ul> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=976682020-05-04T04:37:23ZGo MAEDA
<ul><li><strong>Target version</strong> changed from <i>Candidate for next major release</i> to <i>4.2.0</i></li></ul><p>Setting the target version to 4.2.0.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977042020-05-07T02:08:25ZGo MAEDA
<ul><li><strong>Target version</strong> changed from <i>4.2.0</i> to <i>Candidate for next major release</i></li></ul><p>Thank you for the patch but imports_test.rb fails after applying the patch. Could you look into this?</p>
<pre>
Failure:
RoutingImportsTest#test_imports [/Users/maeda/redmines/trunk/test/test_helper.rb:313]:
The generated path </issues/imports/new> did not match </users/imports/new>.
Expected: "/users/imports/new"
Actual: "/issues/imports/new"
bin/rails test test/integration/routing/imports_test.rb:23
</pre> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977172020-05-08T03:11:05ZTakenori TAKAKItakenory@gmail.com
<ul><li><strong>File</strong> <a href="/attachments/25382">csv_user_import-002.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/25382/csv_user_import-002.patch">csv_user_import-002.patch</a> added</li></ul><p>Thank you, Mr. Maeda, for pointing out the bug.<br />The test code I tried during development was still in the patch...<br />I will re-post a patch that removes the unnecessary test code.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977242020-05-08T06:09:31ZGo MAEDA
<ul><li><strong>Target version</strong> changed from <i>Candidate for next major release</i> to <i>4.2.0</i></li></ul> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977262020-05-08T07:31:20ZMarius BĂLTEANU
<ul></ul><p>Nice feature, I've only one question: Why do you rescue the templates (<code>rescue nil</code>)?</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977422020-05-09T04:09:35ZTakenori TAKAKItakenory@gmail.com
<ul></ul><p>Marius BALTEANU wrote:</p>
<blockquote>
<p>Nice feature, I've only one question: Why do you rescue the templates (<code>rescue nil</code>)?</p>
</blockquote>
<p>For this feature, I thought there was no information that should be displayed using the sidebar partial.<br />So I added `rescue nil` to make the template work with or without the sidebar partial.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977702020-05-12T02:52:53ZGo MAEDA
<ul></ul><p>Takenori TAKAKI wrote:</p>
<blockquote>
<p>Marius BALTEANU wrote:</p>
<blockquote>
<p>Nice feature, I've only one question: Why do you rescue the templates (<code>rescue nil</code>)?</p>
</blockquote>
<p>For this feature, I thought there was no information that should be displayed using the sidebar partial.<br />So I added `rescue nil` to make the template work with or without the sidebar partial.</p>
</blockquote>
<p>How about avoiding catching all exceptions by checking the existence of the partial with <a href="https://api.rubyonrails.org/classes/ActionView/LookupContext/ViewPaths.html#method-i-exists-3F" class="external">ActionView::LookupContext::ViewPaths#exists?</a> ?</p>
<p>I have confirmed that the following experimental code works as expected. Perhaps we can define a helper method like <code>partial_exists?</code> or <code>render_if_exists</code>.</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/app/views/imports/new.html.erb b/app/views/imports/new.html.erb
index e7ca82428..4e9b89ffb 100644
</span><span class="gd">--- a/app/views/imports/new.html.erb
</span><span class="gi">+++ b/app/views/imports/new.html.erb
</span><span class="p">@@ -12,4 +12,4 @@</span>
<p><%= submit_tag l(:label_next).html_safe + " &#187;".html_safe, :name => nil %></p>
<% end %>
-<%= render :partial => "#{import_partial_prefix}_sidebar" %>
<span class="gi">+<%= render :partial => "#{import_partial_prefix}_sidebar" if lookup_context.exists?("#{import_partial_prefix}_sidebar", lookup_context.prefixes, true) %>
</span></code></pre> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=977902020-05-13T06:44:58ZTakenori TAKAKItakenory@gmail.com
<ul><li><strong>File</strong> <a href="/attachments/25416">csv_user_import-003.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/25416/csv_user_import-003.patch">csv_user_import-003.patch</a> added</li></ul><p>Go MAEDA wrote:</p>
<blockquote>
<p>How about avoiding catching all exceptions by checking the existence of the partial with <a href="https://api.rubyonrails.org/classes/ActionView/LookupContext/ViewPaths.html#method-i-exists-3F" class="external">ActionView::LookupContext::ViewPaths#exists?</a> ?</p>
<p>I have confirmed that the following experimental code works as expected. Perhaps we can define a helper method like <code>partial_exists?</code> or <code>render_if_exists</code>.</p>
</blockquote>
<p>I think the proposed code is a more polite implementation.<br />I created a patch that defines ImportsHelper#render_if_exists, how do you like it?</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=978492020-05-17T13:43:46ZMarius BĂLTEANU
<ul></ul><p>The <code>render_if_exists</code> method is much better than <code>rescue nil</code>, but to be honest, I don't see a real value for users to show the issues/time entries sidebar during import wizard, especially after we removed the navigations links from the sidebar (<a class="issue tracker-3 status-5 priority-4 priority-default closed" title="Patch: Move the links (View all issues, Summary, Import) from the Issues section of the issues list side... (Closed)" href="https://www.redmine.org/issues/30294">#30294</a>, #315980). My recommendation is just to remove the sidebar, it's simplify the code.</p>
<p>But if we want to keep the sidebar, then let's render the administration sidebar in order to be consistent with issues/time entries.</p>
<p>Also, the new route added by the patch should be covered by a test in <code>test/integration/routing/imports_test.rb</code>.</p>
<p>Regarding the functionality, I'm wondering If we should support "Generate password" and "Send information to users". From a security/privacy point of view, I think it will nice to import users without defining a default password for them (even if you are an administrator). This can be added later if we agree.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=978502020-05-17T15:55:01ZTakenori TAKAKItakenory@gmail.com
<ul></ul><p>Marius, thank you for your feedback.</p>
<p>Marius BALTEANU wrote:</p>
<blockquote>
<p>The <code>render_if_exists</code> method is much better than <code>rescue nil</code>, but to be honest, I don't see a real value for users to show the issues/time entries sidebar during import wizard, especially after we removed the navigations links from the sidebar (<a class="issue tracker-3 status-5 priority-4 priority-default closed" title="Patch: Move the links (View all issues, Summary, Import) from the Issues section of the issues list side... (Closed)" href="https://www.redmine.org/issues/30294">#30294</a>, #315980). My recommendation is just to remove the sidebar, it's simplify the code.</p>
</blockquote>
<p>I agree with Marius on the value of the sidebar.</p>
<blockquote>
<p>But if we want to keep the sidebar, then let's render the administration sidebar in order to be consistent with issues/time entries.</p>
</blockquote>
<p>I use the 'layout/admin' on the patch because this feature is for admins. Therefore, the current patch already renders the administration sidebar.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=978512020-05-17T16:39:36ZMarius BĂLTEANU
<ul></ul><p>Takenori TAKAKI wrote:</p>
<blockquote>
<p>I agree with Marius on the value of the sidebar.</p>
</blockquote>
<p>Let's wait for Go Maeda feedback on this.</p>
<blockquote>
<p>I use the 'layout/admin' on the patch because this feature is for admins. Therefore, the current patch already renders the administration sidebar.</p>
</blockquote>
<p>You're right, sorry for the mistake.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=978662020-05-18T15:25:47ZGo MAEDA
<ul></ul><p>Marius BALTEANU wrote:</p>
<blockquote>
<p>The <code>render_if_exists</code> method is much better than <code>rescue nil</code>, but to be honest, I don't see a real value for users to show the issues/time entries sidebar during import wizard, especially after we removed the navigations links from the sidebar (<a class="issue tracker-3 status-5 priority-4 priority-default closed" title="Patch: Move the links (View all issues, Summary, Import) from the Issues section of the issues list side... (Closed)" href="https://www.redmine.org/issues/30294">#30294</a>, #315980). My recommendation is just to remove the sidebar, it's simplify the code.</p>
</blockquote>
<p>I agree that it is hard to imagine the situation that users need the content in the sidebar while using the CSV Import Wizard. However, I am not sure if we can remove the sidebar because some third-party plugins may suppose the existence of sidebar or use hooks such as <code>view_issues_sidebar_planning_bottom</code> and <code>view_issues_sidebar_queries_bottom</code>.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=979702020-05-28T04:46:47ZTakenori TAKAKItakenory@gmail.com
<ul></ul><p>Go MAEDA wrote:</p>
<blockquote>
<p>I agree that it is hard to imagine the situation that users need the content in the sidebar while using the CSV Import Wizard. However, I am not sure if we can remove the sidebar because some third-party plugins may suppose the existence of sidebar or use hooks such as <code>view_issues_sidebar_planning_bottom</code> and <code>view_issues_sidebar_queries_bottom</code>.</p>
</blockquote>
<p>I understand that there is a need to discuss the deletion of the sidebar during import wizard. On the other hand, I thought it would be good to discuss separately from the feature addition(Import user accounts from CSV file).</p>
<p>Go Maeda, what do you think?</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=979732020-05-28T07:09:18ZMarius BĂLTEANU
<ul></ul><p>Takenori TAKAKI wrote:</p>
<blockquote>
<p>Go MAEDA wrote:</p>
<blockquote>
<p>I agree that it is hard to imagine the situation that users need the content in the sidebar while using the CSV Import Wizard. However, I am not sure if we can remove the sidebar because some third-party plugins may suppose the existence of sidebar or use hooks such as <code>view_issues_sidebar_planning_bottom</code> and <code>view_issues_sidebar_queries_bottom</code>.</p>
</blockquote>
<p>I understand that there is a need to discuss the deletion of the sidebar during import wizard. On the other hand, I thought it would be good to discuss separately from the feature addition(Import user accounts from CSV file).</p>
<p>Go Maeda, what do you think?</p>
</blockquote>
<p>You're right, let's move the discussion about the sidebar in another ticket.</p>
<p>Regarding your patch, it looks good to me, one minor change that we can do is to move the new method to ApplicationHelper because the method is quite general and it can be used in other pages as well.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=979792020-05-28T10:06:01ZTakenori TAKAKItakenory@gmail.com
<ul><li><strong>File</strong> <a href="/attachments/25483">csv_user_import-004.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/25483/csv_user_import-004.patch">csv_user_import-004.patch</a> added</li></ul><p>Marius BALTEANU wrote:</p>
<blockquote>
<p>You're right, let's move the discussion about the sidebar in another ticket.</p>
</blockquote>
<p>Thanks for your comment and agreement!</p>
<blockquote>
<p>Regarding your patch, it looks good to me, one minor change that we can do is to move the new method to ApplicationHelper because the method is quite general and it can be used in other pages as well.</p>
</blockquote>
<p>I think it's better that way.<br />I made a patch that moved the new method to ApplicationHelper.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=979802020-05-28T13:00:34ZGo MAEDA
<ul></ul><p>Thank you for making the patch even more sophisticated. I think this patch should be merged into the core.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=980062020-05-30T04:54:16ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Closed</i></li><li><strong>Assignee</strong> set to <i>Go MAEDA</i></li><li><strong>Resolution</strong> set to <i>Fixed</i></li></ul><p>Committed the patch. Thank you for writing the patch for this feature.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=980402020-06-02T14:01:14ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>Closed</i> to <i>Reopened</i></li></ul><p><code>user_import_test.rb</code> randomly fails due to missing fixtures. It can be fixed with the following patch.</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/test/unit/user_import_test.rb b/test/unit/user_import_test.rb
index fe1201dff..b790aeb98 100644
</span><span class="gd">--- a/test/unit/user_import_test.rb
</span><span class="gi">+++ b/test/unit/user_import_test.rb
</span><span class="p">@@ -20,6 +20,8 @@</span>
require File.expand_path('../../test_helper', __FILE__)
class UserImportTest < ActiveSupport::TestCase
<span class="gi">+ fixtures :users, :auth_sources, :custom_fields
+
</span> include Redmine::I18n
def setup
</code></pre> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=980502020-06-03T13:13:42ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>Reopened</i> to <i>Closed</i></li></ul><p>Go MAEDA wrote:</p>
<blockquote>
<p><code>user_import_test.rb</code> randomly fails due to missing fixtures. It can be fixed with the following patch.</p>
</blockquote>
<p>Committed the fix in <a class="changeset" title="Add fixtures for UserImportTest (#33102)." href="https://www.redmine.org/projects/redmine/repository/svn/revisions/19802">r19802</a>.</p> Redmine - Feature #33102: Import user accounts from CSV filehttps://www.redmine.org/issues/33102?journal_id=1114332023-11-08T05:57:34ZMischa The Evil
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/36524">Defect #36524</a>: Query Links on Issues and Time Logs Import Sidebars broken</i> added</li></ul>