Feature #28234

Add CSV Import for Time Entries

Added by Gregor Schmidt over 1 year ago. Updated 16 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Importers
Target version:4.1.0
Resolution:Fixed

Description

Preface

The imports controller was obviously built to be used for more than just issue imports. But since Redmine currently only supports issue imports, there are now some places, where assumptions about the issue import are hard-coded.

There are some plugins out there, which add other kinds of CSV imports to Redmine, but none of them seems to use the imports controller. I assume, it's because of the shortcomings mentioned above. I wanted to improve this situation, so I decided to build a time entry importer and - along the way - generalize the imports controller so that it's able to handle more than just issues.

Preparations

The first patch attached extracts the issue related view parts into partials which are determined by convention. They are all prefixed with the imported model name, i.e. _issues.

The issue related model parts are moved to the import model. Those are, e.g. the proper menu item, the authorization or the layout. Strictly speaking they should be located on the controller level, but I did not want to build sub classes of the imports controller for each and every kind of import.

Adding the time entry import

The second patch attached adds the time entry import. It's using the newly introduced extension points to fill in the blanks. To provide a way for the user to find the new import, I've added the timelog/_sidebar partial, similar to the one for issues. This new sidebar partial is now used in the timelog views and it contains a link to the import.

Note: within said partial, I've mis/re-used an i18n key label_time_entries_visibility_all since it was providing the desired text in German and English. I did not want to introduce a new key based on the existing one, since that would have probably caused major merge problems. If desired, I may of course do that anyway.

Closing notes

The second patch may of course also become a plugin. The main point of this patch (series) was it, to allow plugins to reuse the existing plugin infrastructure. Without the first patch though, a plugin author would need copy most of the views and functionality of the imports class, which is undesirable (at least for the plugin author).

Thank you for considering these patches.

0002-Import-time-entries.patch Magnifier - updated version (v2) (16.5 KB) Gregor Schmidt, 2018-03-05 13:09

0001-Generalize-issues-imports.patch Magnifier - updated version (v2) (16.4 KB) Gregor Schmidt, 2018-03-05 13:09

0001-Generalize-issues-imports.patch Magnifier - updated version (v3) (22.6 KB) Gregor Schmidt, 2018-03-12 09:21

0002-Import-time-entries.patch Magnifier - updated version (v3) (16.5 KB) Gregor Schmidt, 2018-03-12 09:21


Related issues

Duplicated by Redmine - Feature #25578: Import spent time Closed

Associated revisions

Revision 18145
Added by Go MAEDA 16 days ago

Generalize issues imports (#28234).

Extend import controller to support arbitrary imports based on Import subclasses. This way, we may add other kinds of imports, by providing some views and a custom import class. This may also be done from within plugins.

Patch by Gregor Schmidt.

Revision 18146
Added by Go MAEDA 16 days ago

Import time entries (#28234).

Patch by Gregor Schmidt.

Revision 18147
Added by Go MAEDA 16 days ago

Update locales (#28213).

History

#1 Updated by Jan from Planio www.plan.io about 1 year ago

  • Target version set to Candidate for next minor release

#2 Updated by Mischa The Evil about 1 year ago

Thanks for sharing this, Gregor. I really like these patches (especially the first one).
I'd target this issue against Unplanned backlogs – along with #28198 and #28213, but since Jan already set the target to Candidate for next minor release, I'll leave it as-is.

#3 Updated by Marius BALTEANU about 1 year ago

#4 Updated by Gregor Schmidt about 1 year ago

In my initial version, I've missed the JavaScript-based updates of the form. This is fixed in the new patch files, which are attached to this note. The initial ones are now obsolete.

#5 Updated by Go MAEDA about 1 year ago

  • File deleted (0002-Import-time-entries.patch)

#6 Updated by Go MAEDA about 1 year ago

  • File deleted (0001-Generalize-issues-imports.patch)

#7 Updated by Go MAEDA about 1 year ago

  • Status changed from New to Needs feedback
  • Assignee set to Gregor Schmidt

I tried out the import time entries feature and it works well. Thank you for the patch.

But it seems that the issue import feature is broken after applying the patch. Maybe the patch missing some view partials. Could you check the patch?

$ ruby test/functional/imports_controller_test.rb
Run options: --seed 16739

# Running:

F

Failure:
ImportsControllerTest#test_get_mapping_should_display_mapping_form [test/functional/imports_controller_test.rb:154]:
Expected response to be a <2XX: success>, but was a <404: Not Found>
<?pre>

#8 Updated by Gregor Schmidt about 1 year ago

Thank you very much for taking a look at these changes.

I'm afraid, I cannot reproduce the problem. I've used git am to re-apply the patches to my local git clone of https://github.com/redmine/redmine and the tests work fine.

I can see three potential causes

  1. git am does different things than patch -p0. Therefore maybe the rename of one of the files, wasn't applied properly. Do you have a file called app/views/imports/_issues_fields_mapping.html.erb? If you have this file, then this is not the problem.
  2. The functional test may be missing some fixture definition. Do the test work, when you run them all together?
  3. I don't have all the optional Redmine dependencies installed (e.g. ImageMagick, ldap things and so on) and I'm running the tests with sqlite. Maybe one of the optional dependencies causes the problem?

I think, the first is the most likely one. If you can confirm this, I'll need to figure out how to create fully patch-compatible diff files.

Do you maybe have additional information in you log/test.log?

#9 Updated by Go MAEDA about 1 year ago

  • Status changed from Needs feedback to New
  • Assignee deleted (Gregor Schmidt)

Gregor Schmidt wrote:

  1. git am does different things than patch -p0. Therefore maybe the rename of one of the files, wasn't applied properly. Do you have a file called app/views/imports/_issues_fields_mapping.html.erb? If you have this file, then this is not the problem.

You are right. I applied the patch with patch -p1. Applying the patch with git am solved the problem. I am sorry for bothering you and thank you so much for kindly advising me about it.

With patch -p1, I got the following error:

  Rendering imports/mapping.html.erb within layouts/base
  Rendered imports/_issues_mapping.html.erb (4.4ms)
  Rendered imports/mapping.html.erb within layouts/base (8.0ms)
Missing template, responding with 404: Missing partial imports/_issues_fields_mapping, application/_issues_fields_mapping with {:locale=>[:en], :formats=>[:html], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :rsb]}. Searched in:
...

#10 Updated by Gregor Schmidt about 1 year ago

Attached you may find an updated version of the patch files. This time they should be fully compatible with the patch command.

#11 Updated by Go MAEDA about 1 year ago

  • Target version changed from Candidate for next minor release to 4.1.0

LGTM, setting target version to 4.1.0.

#12 Updated by Go MAEDA 8 months ago

  • Assignee set to Jean-Philippe Lang

#13 Updated by Go MAEDA 17 days ago

  • Assignee changed from Jean-Philippe Lang to Go MAEDA

I think there is no reason not to deliver this change in 4.1.0. According to the article https://support.plan.io/news/141, Planio deployed this change a year ago. This code has been tested in a production environment for a year and maybe has not encountered any major problems.

I will commit the patches in a few days.

#14 Updated by Jean-Philippe Lang 17 days ago

Sounds good to me.

#15 Updated by Go MAEDA 16 days ago

  • Status changed from New to Closed
  • Resolution set to Fixed

Committed the patches. Thank you for extracting the great feature from Planio.

Jean-Philippe Lang wrote:

Sounds good to me.

Thank you for your feedback!

#16 Updated by Marius BALTEANU 16 days ago

Go MAEDA wrote:

Committed the patches. Thank you for extracting the great feature from Planio.

Great news!

Also available in: Atom PDF