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

Defect #26537

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

Added by Marius BALTEANU almost 3 years ago. Updated over 1 year ago.

Status:

Closed

Start date:

Priority:

Normal

Due date:

Assignee:

Jean-Philippe Lang

% Done:

Category:

Time tracking

Target version:

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.

0001-show-project-column-by-default-in-global-time-entry-.patch (5.06 KB) Marius BALTEANU, 2018-06-26 08:04

Related issues

Associated revisions

Revision 17503
Added by Jean-Philippe Lang over 1 year ago

Column Project is no longer added by default to the list of default columns for time entries (#26537).

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA almost 3 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'
.
.
.

#2 Updated by Marius BALTEANU almost 3 years ago

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

#3 Updated by Go MAEDA almost 3 years ago

Assignee set to Marius BALTEANU

Marius BALTEANU wrote:

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

Done.

#4 Updated by Marius BALTEANU almost 3 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.

#5 Updated by Marius BALTEANU almost 3 years ago

File deleted (~~show_column_project_by_default_time_entries_index.patch~~)

#6 Updated by Marius BALTEANU almost 3 years ago

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

#7 Updated by Go MAEDA almost 3 years ago

Target version set to 4.0.0

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

#8 Updated by Marius BALTEANU almost 2 years ago

Assignee deleted (~~Marius BALTEANU~~)

#9 Updated by Jean-Philippe Lang almost 2 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.

#10 Updated by Marius BALTEANU almost 2 years ago

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

#11 Updated by Marius BALTEANU almost 2 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.

#12 Updated by Marius BALTEANU almost 2 years ago

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

#13 Updated by Marius BALTEANU almost 2 years ago

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

#14 Updated by Marius BALTEANU almost 2 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.