Skip to content

Commit 05e68f8

Browse files
ulfertsulferts
committed
take heed of activity inheritance
1 parent 4eee574 commit 05e68f8

7 files changed

Lines changed: 249 additions & 43 deletions

File tree

‎app/models/queries/time_entries/filters/activity_filter.rb‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,12 @@
3131
class Queries::TimeEntries::Filters::ActivityFilter < Queries::TimeEntries::Filters::TimeEntryFilter
3232
def allowed_values
3333
@allowed_values ||= begin
34-
::TimeEntryActivity.pluck(:name, :id)
34+
# To mask the internal complexity of time entries and to
35+
# allow filtering by a combined value only shared activities are
36+
# valid values
37+
::TimeEntryActivity
38+
.shared
39+
.pluck(:name, :id)
3540
end
3641
end
3742

@@ -42,4 +47,28 @@ def type
4247
def self.key
4348
:activity_id
4449
end
50+
51+
def where
52+
# Because the project specific activity is used for storing the time entry,
53+
# we have to deduce the actual filter value which is the id of all the provided activities' children.
54+
# But when the activity itself is already shared, we use that value.
55+
db_values = child_values
56+
.or(shared_values)
57+
.pluck(:id)
58+
59+
operator_strategy.sql_for_field(db_values, self.class.model.table_name, self.class.key)
60+
end
61+
62+
private
63+
64+
def child_values
65+
TimeEntryActivity
66+
.where(parent_id: values)
67+
end
68+
69+
def shared_values
70+
TimeEntryActivity
71+
.shared
72+
.where(id: values)
73+
end
4574
end

‎app/services/time_entries/create_service.rb‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
module TimeEntries
3232
class CreateService
3333
include Concerns::Contracted
34+
include SharedMixin
3435

3536
attr_reader :user
3637

@@ -62,13 +63,5 @@ def assign_defaults(time_entry)
6263
time_entry.hours = nil if time_entry.hours&.zero?
6364
time_entry.project ||= time_entry.work_package.project if time_entry.work_package
6465
end
65-
66-
def use_project_activity(time_entry)
67-
if time_entry.activity&.shared? && time_entry.project
68-
project_activity = time_entry.project.time_entry_activities.find_by(parent_id: time_entry.activity_id) ||
69-
time_entry.activity
70-
time_entry.activity = project_activity
71-
end
72-
end
7366
end
7467
end

‎app/services/time_entries/shared_mixin.rb‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#-- encoding: UTF-8
2+
3+
#-- copyright
4+
# OpenProject is a project management system.
5+
# Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
6+
#
7+
# This program is free software; you can redistribute it and/or
8+
# modify it under the terms of the GNU General Public License version 3.
9+
#
10+
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
11+
# Copyright (C) 2006-2017 Jean-Philippe Lang
12+
# Copyright (C) 2010-2013 the ChiliProject Team
13+
#
14+
# This program is free software; you can redistribute it and/or
15+
# modify it under the terms of the GNU General Public License
16+
# as published by the Free Software Foundation; either version 2
17+
# of the License, or (at your option) any later version.
18+
#
19+
# This program is distributed in the hope that it will be useful,
20+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
21+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
22+
# GNU General Public License for more details.
23+
#
24+
# You should have received a copy of the GNU General Public License
25+
# along with this program; if not, write to the Free Software
26+
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
27+
#
28+
# See doc/COPYRIGHT.rdoc for more details.
29+
#++
30+
31+
module TimeEntries
32+
module SharedMixin
33+
def use_project_activity(time_entry)
34+
if time_entry.activity&.shared? && time_entry.project
35+
project_activity = time_entry.project.time_entry_activities.find_by(parent_id: time_entry.activity_id) ||
36+
time_entry.activity
37+
38+
time_entry.activity = project_activity
39+
end
40+
end
41+
end
42+
end

‎app/services/time_entries/update_service.rb‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,32 +28,39 @@
2828
# See docs/COPYRIGHT.rdoc for more details.
2929
#++
3030

31-
class TimeEntries::UpdateService
32-
include Concerns::Contracted
33-
attr_accessor :user, :time_entry
34-
35-
def initialize(user:, time_entry:)
36-
self.user = user
37-
self.time_entry = time_entry
38-
self.contract_class = TimeEntries::UpdateContract
39-
end
31+
module TimeEntries
32+
class UpdateService
33+
include Concerns::Contracted
34+
include SharedMixin
4035

41-
def call(attributes: {})
42-
set_attributes attributes
36+
attr_accessor :user,
37+
:time_entry
4338

44-
success, errors = validate_and_save(time_entry, user)
45-
ServiceResult.new success: success, errors: errors, result: time_entry
46-
end
39+
def initialize(user:, time_entry:)
40+
self.user = user
41+
self.time_entry = time_entry
42+
self.contract_class = TimeEntries::UpdateContract
43+
end
44+
45+
def call(attributes: {})
46+
set_attributes attributes
47+
48+
success, errors = validate_and_save(time_entry, user)
49+
ServiceResult.new success: success, errors: errors, result: time_entry
50+
end
51+
52+
private
4753

48-
private
54+
def set_attributes(attributes)
55+
time_entry.attributes = attributes
4956

50-
def set_attributes(attributes)
51-
time_entry.attributes = attributes
57+
##
58+
# Update project context if moving time entry
59+
if time_entry.work_package && time_entry.work_package_id_changed?
60+
time_entry.project_id = time_entry.work_package.project_id
61+
end
5262

53-
##
54-
# Update project context if moving time entry
55-
if time_entry.work_package && time_entry.work_package_id_changed?
56-
time_entry.project_id = time_entry.work_package.project_id
63+
use_project_activity(time_entry)
5764
end
5865
end
5966
end

‎docs/api/apiv3/endpoints/time_entries.apib‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ Lists time entries. The time entries returned depend on the filters provided and
209209
210210
+ pageSize (optional, integer, `25`) ... Number of elements to display per page.
211211
212-
+ sortBy = ["spent_on", "asc"] (optional, string, `[["speont_on", "asc"]]`) ... JSON specifying sort criteria.
212+
+ sortBy = ["spent_on", "asc"] (optional, string, `[["spent_on", "asc"]]`) ... JSON specifying sort criteria.
213213
Accepts the same format as returned by the [queries](#queries) endpoint. Currently supported sorts are:
214214
+ id: Sort by primary key
215215
+ hours: Sort by logged hours

‎spec/models/queries/time_entries/filters/activity_filter_spec.rb‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,33 @@
3333
describe Queries::TimeEntries::Filters::ActivityFilter, type: :model do
3434
let(:time_entry_activity1) { FactoryBot.build_stubbed(:time_entry_activity) }
3535
let(:time_entry_activity2) { FactoryBot.build_stubbed(:time_entry_activity) }
36-
let(:plucked) do
37-
[time_entry_activity1, time_entry_activity2].map { |x| [x.name, x.id] }
36+
let(:activities) { [time_entry_activity1, time_entry_activity2] }
37+
let(:plucked_allowed_values) do
38+
activities.map { |x| [x.name, x.id] }
3839
end
3940

4041
before do
4142
allow(::TimeEntryActivity)
42-
.to receive_message_chain(:pluck)
43+
.to receive_message_chain(:shared)
44+
.and_return(activities)
45+
46+
allow(::TimeEntryActivity)
47+
.to receive_message_chain(:where, :or)
48+
.and_return(activities)
49+
50+
allow(activities)
51+
.to receive(:where)
52+
.and_return(activities)
53+
54+
allow(activities)
55+
.to receive(:pluck)
56+
.with(:id)
57+
.and_return(activities.map(&:id))
58+
59+
allow(activities)
60+
.to receive(:pluck)
4361
.with(:name, :id)
44-
.and_return(plucked)
62+
.and_return(activities.map { |x| [x.name, x.id] })
4563
end
4664

4765
it_behaves_like 'basic query filter' do
@@ -51,14 +69,14 @@
5169

5270
describe '#allowed_values' do
5371
it 'is a list of the possible values' do
54-
expect(instance.allowed_values).to match_array(plucked)
72+
expect(instance.allowed_values).to match_array(activities.map { |x| [x.name, x.id] })
5573
end
5674
end
57-
end
5875

59-
it_behaves_like 'list_optional query filter' do
60-
let(:attribute) { :activity_id }
61-
let(:model) { TimeEntry }
62-
let(:valid_values) { [time_entry_activity1.id.to_s] }
76+
it_behaves_like 'list_optional query filter' do
77+
let(:attribute) { :activity_id }
78+
let(:model) { TimeEntry }
79+
let(:valid_values) { activities.map { |a| a.id.to_s } }
80+
end
6381
end
6482
end

‎spec/requests/api/v3/time_entry_resource_spec.rb‎

Lines changed: 119 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,95 @@
244244
end
245245
end
246246

247+
context 'filtering by global activity' do
248+
let(:project_activity) do
249+
FactoryBot.create(:time_entry_activity,
250+
parent: activity,
251+
project: project)
252+
end
253+
let(:other_project_activity) do
254+
FactoryBot.create(:time_entry_activity,
255+
parent: activity,
256+
project: other_project)
257+
end
258+
let(:another_project_activity) do
259+
FactoryBot.create(:time_entry_activity,
260+
parent: FactoryBot.create(:time_entry_activity),
261+
project: project)
262+
end
263+
let(:shared_activity) do
264+
FactoryBot.create(:time_entry_activity,
265+
parent: nil)
266+
end
267+
let!(:time_entry) do
268+
FactoryBot.create(:time_entry,
269+
project: project,
270+
work_package: work_package,
271+
user: current_user,
272+
activity: project_activity)
273+
end
274+
let!(:other_time_entry) do
275+
FactoryBot.create(:time_entry,
276+
project: other_project,
277+
work_package: other_work_package,
278+
user: current_user,
279+
activity: other_project_activity)
280+
end
281+
let!(:another_time_entry) do
282+
FactoryBot.create(:time_entry,
283+
project: project,
284+
work_package: work_package,
285+
user: current_user,
286+
activity: another_project_activity)
287+
end
288+
let!(:shared_time_entry) do
289+
FactoryBot.create(:time_entry,
290+
project: project,
291+
work_package: work_package,
292+
user: current_user,
293+
activity: shared_activity)
294+
end
295+
296+
before do
297+
FactoryBot.create(:member,
298+
roles: [role],
299+
project: other_project,
300+
user: current_user)
301+
get path
302+
end
303+
304+
let(:path) do
305+
filter = [
306+
{
307+
'activity_id' => {
308+
'operator' => '=',
309+
'values' => [activity.id, shared_activity.id]
310+
}
311+
}
312+
]
313+
314+
"#{api_v3_paths.time_entries}?#{{ filters: filter.to_json }.to_query}&sortBy=#{[%w(id asc)].to_json}"
315+
end
316+
317+
it 'contains only the filtered time entries in the response' do
318+
expect(subject.body)
319+
.to be_json_eql('3')
320+
.at_path('total')
321+
322+
expect(subject.body)
323+
.to be_json_eql(time_entry.id.to_json)
324+
.at_path('_embedded/elements/0/id')
325+
326+
expect(subject.body)
327+
.to be_json_eql(other_time_entry.id.to_json)
328+
.at_path('_embedded/elements/1/id')
329+
330+
expect(subject.body)
331+
.to be_json_eql(shared_time_entry.id.to_json)
332+
.at_path('_embedded/elements/2/id')
333+
end
334+
end
335+
247336
context 'invalid filter' do
248337
let(:path) do
249338
filter = [{ 'bogus' => {
@@ -333,7 +422,7 @@
333422
}
334423
}
335424
end
336-
let(:additional_setup) { ->{} }
425+
let(:additional_setup) { -> {} }
337426

338427
before do
339428
work_package
@@ -445,14 +534,21 @@
445534

446535
let(:params) do
447536
{
448-
"hours": 'PT10H'
537+
"hours": 'PT10H',
538+
"activity": {
539+
"href": api_v3_paths.time_entries_activity(activity.id)
540+
}
449541
}
450542
end
451543

544+
let(:additional_setup) { -> {} }
545+
452546
before do
453547
time_entry
454548
custom_value
455549

550+
additional_setup.call
551+
456552
patch path, params.to_json, 'CONTENT_TYPE' => 'application/json'
457553
end
458554

@@ -461,6 +557,8 @@
461557

462558
time_entry.reload
463559
expect(time_entry.hours).to eq 10
560+
561+
expect(time_entry.activity).to eq activity
464562
end
465563

466564
context 'when lacking permissions' do
@@ -472,6 +570,25 @@
472570
end
473571
end
474572

573+
context 'when sending an activity the project overrides' do
574+
let(:project_activity) do
575+
params = activity.attributes.except('id')
576+
params['parent_id'] = activity.id
577+
project.create_time_entry_activity_if_needed(params)
578+
579+
project.time_entry_activities.first
580+
end
581+
582+
let(:additional_setup) { -> { project_activity } }
583+
584+
it 'creates the time entry with the project activity' do
585+
time_entry.reload
586+
587+
expect(time_entry.activity)
588+
.to eql project_activity
589+
end
590+
end
591+
475592
context 'when sending invalid params' do
476593
let(:params) do
477594
{

0 commit comments

Comments
 (0)