Defect #26537
Column Project is not longer added by default to the list of default columns for time entries
Status: | Closed | Start date: | ||
---|---|---|---|---|
Priority: | Normal | Due date: | ||
Assignee: | % Done: | 0% | ||
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.
Related issues
Associated revisions
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 5 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 5 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 5 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 5 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 5 years ago
- File deleted (
show_column_project_by_default_time_entries_index.patch)
#6
Updated by Marius BALTEANU almost 5 years ago
- Related to Feature #26356: Time entry list: set default column options added
#7
Updated by Go MAEDA almost 5 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 4 years ago
- Assignee deleted (
Marius BALTEANU)
#9
Updated by Jean-Philippe Lang almost 4 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 4 years ago
- File deleted (
26537_show_column_project_by_default_time_entries_index.patch)
#11
Updated by Marius BALTEANU almost 4 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 4 years ago
- File deleted (
0001-show-project-column-by-default-in-global-time-entry-.patch)
#13
Updated by Marius BALTEANU almost 4 years ago
I had an error in my previous patch and I've removed it for now.
#14
Updated by Marius BALTEANU almost 4 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.
#15
Updated by Marius BALTEANU almost 4 years ago
- File deleted (
0001-show-project-column-by-default-in-global-time-entry-.patch)
#16
Updated by Marius BALTEANU almost 4 years ago
- File 0001-show-project-column-by-default-in-global-time-entry-.patch added
#17
Updated by Marius BALTEANU almost 4 years ago
- File deleted (
0001-show-project-column-by-default-in-global-time-entry-.patch)
#18
Updated by Marius BALTEANU almost 4 years ago
Sorry for the noise, now is the correct patch attached.
#19
Updated by Jean-Philippe Lang over 3 years ago
- Status changed from New to Closed
- Resolution set to Fixed
Committed, thanks.
#20
Updated by Marius BALTEANU over 3 years ago
- Target version deleted (
4.0.0)
I'm removing this ticket from the Changelog because no stable versions were been affected by this problem.