Defect #33914

Even if the default value of Activities (time tracking) is set, it may not be reflected.

Added by Mizuki ISHIKAWA almost 2 years ago. Updated 13 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Marius BALTEANU% Done:

0%

Category:Time tracking
Target version:4.2.7
Resolution:Fixed Affected version:

Description

How to reproduce the problem:
1. bundle exec rake db:fixtures:load
2. Open /projects/ecookbook/time_entries/new and you can see that "Development" is the default Activity choice.
3. Go to /projects/ecookbook/settings/activities and click Save button. At this time TimeEntryActivity.count changes from 4 to 8.
4. When you open /projects/ecookbook/time_entries/new, the default Activity choice is "--- Please select ---".

If you click the Save button in /projects/ecookbook/settings/activities, project-specific activity data will be created based on the original activity data.
If the underlying activity data is the default, then the project-specific activity data based on it should also be the default.

fix-33914.patch Magnifier (3.17 KB) Mizuki ISHIKAWA, 2020-08-27 10:19

Associated revisions

Revision 21668
Added by Marius BALTEANU 13 days ago

Fix time entry activity is not set as default when the default activity exists as project specific activity (#33914).

Patch by Mizuki ISHIKAWA.

Revision 21669
Added by Marius BALTEANU 13 days ago

Refactor TimeEntryActivity.default() method (#33914).

Patch by Holger Just.

Revision 21670
Added by Marius BALTEANU 13 days ago

Merged r21668 and r21669 to 5.0-stable (#33914).

Revision 21671
Added by Marius BALTEANU 13 days ago

Merged r21668 and r21669 to 4.2-stable (#33914).

History

#1 Updated by Mizuki ISHIKAWA almost 2 years ago

I attach the patch.

#3 Updated by Go MAEDA over 1 year ago

  • Status changed from New to Confirmed

#4 Updated by Mizuki ISHIKAWA 20 days ago

Is it hard to commit to this patch?
I can improve the patch if there are any problems.

#5 Updated by Marius BALTEANU 13 days ago

  • Assignee set to Marius BALTEANU
  • Target version set to 4.2.7

#6 Updated by Marius BALTEANU 13 days ago

  • Status changed from Confirmed to Resolved
  • Resolution set to Fixed

#7 Updated by Holger Just 13 days ago

In TimeEntryActivity.default, all of the project's activities are already loaded from the database in the if statement. As such, the trailing find_by is redundant and should be replaced by Ruby-only filtering to avoid running another SQL query to load data we already have.

The trailing line of that method could thus be replaced by the following code with matching behavior to avoid the additional SQL query:

project.activities.detect { |activity| activity.parent_id == default_activity.id }

Also, it is not guaranteed that we have any default activity at all as admins could deselect the default flag on all activities. Thus would result in the super call to return nil which in turn would result in an exception in the final line of the overwritten default method. As such, we also have to check for default_activity.nil?. The following method should address both of my points:

def self.default(project=nil)
  default_activity = super()

  if default_activity.nil? || project.nil? || project.activities.blank? || project.activities.include?(default_activity)
    return default_activity
  end

  project.activities.detect { |activity| activity.parent_id == default_activity.id }
end

#8 Updated by Marius BALTEANU 13 days ago

  • Status changed from Resolved to Closed

Thanks Holger for your quick review on this. I've committed both patches.

#9 Updated by Mizuki ISHIKAWA 13 days ago

Thank you very much.

Also available in: Atom PDF