Defect #26537: Column Project is not longer added by default to the list of default columns for time entries - Redmine

Custom queries

Documentation issues
Open defects
Open features
Patch queue
Plugin issues
Translation patches

Actions

Copy link

Defect #26537

closed

Column Project is not longer added by default to the list of default columns for time entries

Added by Marius BĂLTEANU over 7 years ago. Updated about 6 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Jean-Philippe Lang

Category:

Time tracking

Target version:

Start date:

Due date:

% Done:

Estimated time:

Resolution:

Fixed

Affected version:

Description

After r16814, the column "Project" is no longer added by default to the list of default columns for time entries in the global view.

To reproduce:

Go to "/time_entries"
Observe that the column Project is missing

For issues#index and before this commit, the columns Project was shown by default in the global view.

Attached is a patch that fixes this.

Files

0001-show-project-column-by-default-in-global-time-entry-.patch (5.06 KB) 0001-show-project-column-by-default-in-global-time-entry-.patch

Marius BĂLTEANU, 2018-06-26 08:04

Related issues

Related to Redmine - Feature #26356: Time entry list: set default column options

Closed

Jean-Philippe Lang

Actions

Issue # Delay: days Cancel

History
Notes
Property changes
Associated revisions

Actions

Copy link

Updated by Go MAEDA over 7 years ago

Thank you for the patch but the following error occurs if Setting.time_entry_list_defaults[:column_names] is nil. The value is nil in a fresh installation.

Could you fix the patch?

ActionView::Template::Error (undefined method `map' for nil:NilClass):
     6:   <span>
     7:       <%= label_tag available_tag_id, l(:description_available_columns) %>
     8:       <%= select_tag 'available_columns',
     9:               options_for_select(query_available_inline_columns_options(query)),
    10:               :id => available_tag_id,
    11:               :multiple => true, :size => 10,
    12:               :ondblclick => "moveOptions(this.form.#{available_tag_id}, this.form.#{selected_tag_id});" %>

app/models/time_entry_query.rb:102:in `default_columns_names'
app/models/query.rb:677:in `columns'
app/helpers/queries_helper.rb:109:in `query_available_inline_columns_options'
.
.
.

Actions

Copy link

Updated by Marius BĂLTEANU over 7 years ago

Thanks for finding this issue, I'll take a look these days. Can you assign this ticket to me, please?

Actions

Copy link

Updated by Go MAEDA over 7 years ago

Assignee set to Marius BĂLTEANU

Marius BALTEANU wrote:

I'll take a look these days. Can you assign this ticket to me, please?

Done.

Actions

Copy link

Updated by Marius BĂLTEANU over 7 years ago

File 26537_show_column_project_by_default_time_entries_index.patch added

I've fixed the issue by using the same logic implementation from issue default columns.

Actions

Copy link

Updated by Marius BĂLTEANU over 7 years ago

File deleted (~~show_column_project_by_default_time_entries_index.patch~~)

Actions

Copy link

Updated by Marius BĂLTEANU over 7 years ago

Related to Feature #26356: Time entry list: set default column options added

Actions

Copy link

Updated by Go MAEDA over 7 years ago

Target version set to 4.0.0

LGTM. Setting target version to 4.0.0.
Thank you for fixing this issue.

Actions

Copy link

Updated by Marius BĂLTEANU over 6 years ago

Assignee deleted (~~Marius BĂLTEANU~~)

Actions

Copy link

Updated by Jean-Philippe Lang over 6 years ago

This should be fixed indeed but we should not alter application settings for that.
Defaults for time entries queries are stored in a single setting time_entry_list_defaults and I prefer to keep it like that.

Actions

Copy link

#10

Updated by Marius BĂLTEANU over 6 years ago

File deleted (~~26537_show_column_project_by_default_time_entries_index.patch~~)

Actions

Copy link

#11

Updated by Marius BĂLTEANU over 6 years ago

File 0001-show-project-column-by-default-in-global-time-entry-.patch added
Assignee set to Jean-Philippe Lang

Another fix attached without altering application settings. The patch includes also a fix for a missing fixture for #28391.

Jean-Phillipe, if the proposed fix is still not good, I think we can get rid of the methods default_columns_names and default_totalable_names and add the project column to the default list (if is not in a project context) in the build_from_params method. Another ideas I don't have.

Actions

Copy link

#12

Updated by Marius BĂLTEANU over 6 years ago

File deleted (~~0001-show-project-column-by-default-in-global-time-entry-.patch~~)

Actions

Copy link

#13

Updated by Marius BĂLTEANU over 6 years ago

I had an error in my previous patch and I've removed it for now.

Actions

Copy link

#14

Updated by Marius BĂLTEANU over 6 years ago

File 0001-show-project-column-by-default-in-global-time-entry-.patch added

Here is the updated patch.

Jean-Philippe, some mentions regarding my patch:
- I've removed the default columns defined in the default_columns_names and default_totalable_names methods because I found them confusing to have them defined there and also in the application settings. I preferred to keep them only in the default application settings.
- I've added an empty hidden tag in app/views/settings/_timelog.html.erb in order to prevent saving empty columns list from UI
- I've fixed the tests test_index_with_default_query_setting by adding to the settings the key totalable_names which is required.

I think that we can improve the current implementation by validating the presence of the keys totalable_names and column_names for setting time_entry_list_defaults in the "Setting" model. In this way, users will receive a validation error when they will try to save an empty columns list instead of the current message "Successful update.", but without actually saving.

Also, maybe is a good idea after we have the final implementation for this feature to move the settings for issues default list (stored now in 2 settings) to one setting and to have the same implementations.

About my previous note, I'm not sure anymore if the solution with adding the project in the build_from_params method is an option.

Actions

Copy link

#15