Commit 7cfe0608 authored by Sean McGivern's avatar Sean McGivern

Merge branch '55249-change-pa-merged-at-parameters' into 'master'

Rename productivity analytics date parameters

Closes #55249

See merge request gitlab-org/gitlab!22377
parents 249e94f7 9860665e
......@@ -26,12 +26,12 @@ export default () => {
authorUsername,
labelName,
milestoneTitle,
mergedAtAfter,
mergedAtBefore,
mergedAfter,
mergedBefore,
} = container.dataset;
const mergedAtAfterDate = new Date(mergedAtAfter);
const mergedAtBeforeDate = new Date(mergedAtBefore);
const mergedAfterDate = new Date(mergedAfter);
const mergedBeforeDate = new Date(mergedBefore);
const { endpoint, emptyStateSvgPath, noAccessSvgPath } = appContainer.dataset;
const minDate = timeframeContainer.dataset.startDate
......@@ -42,8 +42,8 @@ export default () => {
let project = null;
let initialData = {
mergedAtAfter: mergedAtAfterDate,
mergedAtBefore: mergedAtBeforeDate,
mergedAfter: mergedAfterDate,
mergedBefore: mergedBeforeDate,
minDate,
};
......@@ -152,8 +152,8 @@ export default () => {
return h(DateRange, {
props: {
show: this.groupNamespace !== null,
startDate: mergedAtAfterDate,
endDate: mergedAtBeforeDate,
startDate: mergedAfterDate,
endDate: mergedBeforeDate,
minDate,
},
on: {
......
......@@ -80,12 +80,10 @@ export const setFilters = (
export const setDateRange = ({ commit, dispatch }, { startDate, endDate }) => {
commit(types.SET_DATE_RANGE, { startDate, endDate });
const mergedAtAfter = `${dateFormat(startDate, dateFormats.isoDate)}${beginOfDayTime}`;
const mergedAtBefore = `${dateFormat(endDate, dateFormats.isoDate)}${endOfDayTime}`;
const mergedAfter = `${dateFormat(startDate, dateFormats.isoDate)}${beginOfDayTime}`;
const mergedBefore = `${dateFormat(endDate, dateFormats.isoDate)}${endOfDayTime}`;
historyPushState(
setUrlParams({ merged_at_after: mergedAtAfter, merged_at_before: mergedAtBefore }),
);
historyPushState(setUrlParams({ merged_after: mergedAfter, merged_before: mergedBefore }));
dispatch('charts/resetMainChartSelection', true, { root: true });
......
......@@ -14,8 +14,8 @@ import { dateFormats } from '../../../../shared/constants';
* author_username: 'author',
* milestone_title: 'my milestone',
* label_name: ['my label', 'yet another label'],
* merged_at_after: '2019-06-11T00:00:00Z'
* merged_at_before: '2019-09-09T23:59:59Z'
* merged_after: '2019-06-11T00:00:00Z'
* merged_before: '2019-09-09T23:59:59Z'
* }
*
*/
......@@ -31,7 +31,7 @@ export const getCommonFilterParams = (state, getters) => chartKey => {
} = state;
// for the scatterplot we need to query the API with a date prior to the selected start date
const mergedAtAfterDate =
const mergedAfterDate =
chartKey && chartKey === chartKeys.scatterplot
? dateFormat(getters.scatterplotStartDate, dateFormats.isoDate)
: dateFormat(startDate, dateFormats.isoDate);
......@@ -42,8 +42,8 @@ export const getCommonFilterParams = (state, getters) => chartKey => {
author_username: authorUsername,
milestone_title: milestoneTitle,
label_name: labelName,
merged_at_after: `${mergedAtAfterDate}${beginOfDayTime}`,
merged_at_before: `${dateFormat(endDate, dateFormats.isoDate)}${endOfDayTime}`,
merged_after: `${mergedAfterDate}${beginOfDayTime}`,
merged_before: `${dateFormat(endDate, dateFormats.isoDate)}${endOfDayTime}`,
};
};
......
......@@ -9,8 +9,8 @@ export default {
authorUsername = null,
labelName = [],
milestoneTitle = null,
mergedAtAfter,
mergedAtBefore,
mergedAfter,
mergedBefore,
minDate,
},
) {
......@@ -19,8 +19,8 @@ export default {
state.authorUsername = authorUsername;
state.labelName = labelName;
state.milestoneTitle = milestoneTitle;
state.startDate = mergedAtAfter;
state.endDate = mergedAtBefore;
state.startDate = mergedAfter;
state.endDate = mergedBefore;
state.minDate = minDate;
},
[types.SET_GROUP_NAMESPACE](state, groupNamespace) {
......
......@@ -85,8 +85,8 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
def allowed_request_params
params.permit(
:merged_at_after,
:merged_at_before,
:merged_after,
:merged_before,
:author_username,
:milestone_title,
label_name: []
......
......@@ -6,7 +6,7 @@ class ProductivityAnalyticsFinder < MergeRequestsFinder
end
def self.scalar_params
@scalar_params ||= super + [:merged_at_before, :merged_at_after]
@scalar_params ||= super + [:merged_before, :merged_after]
end
def filter_items(_items)
......@@ -34,7 +34,7 @@ class ProductivityAnalyticsFinder < MergeRequestsFinder
# rubocop: disable CodeReuse/ActiveRecord
def by_merged_at(items)
return items unless params[:merged_at_after] || params[:merged_at_before]
return items unless params[:merged_after] || params[:merged_before]
items = items.joins(:metrics)
items = items.where(metrics_table[:merged_at].gteq(merged_at_between[:from])) if merged_at_between[:from]
......@@ -46,7 +46,7 @@ class ProductivityAnalyticsFinder < MergeRequestsFinder
def merged_at_between
@merged_at_between ||= begin
boundaries = { from: params[:merged_at_after], to: params[:merged_at_before] }
boundaries = { from: params[:merged_after], to: params[:merged_before] }
if ProductivityAnalytics.start_date && ProductivityAnalytics.start_date > boundaries[:from]
boundaries[:from] = ProductivityAnalytics.start_date
......
---
title: Rename productivity analytics date parameters
merge_request: 22377
author:
type: changed
......@@ -10,24 +10,24 @@ module Analytics
attr_writer :label_name
attribute :merged_at_after, :datetime
attribute :merged_at_before, :datetime
attribute :merged_after, :datetime
attribute :merged_before, :datetime
attribute :author_username, :string
attribute :milestone_title, :string
attr_accessor :group, :project
validates :group, presence: true
validates :merged_at_after, presence: true
validates :merged_at_before, presence: true
validates :merged_after, presence: true
validates :merged_before, presence: true
validate :validate_merged_at_after_is_earlier_than_merged_at_before
validate :validate_merged_at_before
validate :validate_merged_at_after
validate :validate_merged_after_is_earlier_than_merged_before
validate :validate_merged_before
validate :validate_merged_after
def initialize(params = {})
params[:merged_at_before] ||= Date.today.at_end_of_day
params[:merged_at_after] ||= default_merged_at_after
params[:merged_before] ||= Date.today.at_end_of_day
params[:merged_after] ||= default_merged_after
super(params)
end
......@@ -43,13 +43,13 @@ module Analytics
attrs[:author_username] = author_username
attrs[:label_name] = label_name.any? ? label_name.join(',') : nil
attrs[:milestone_title] = milestone_title
attrs[:merged_at_after] = merged_at_after.iso8601
attrs[:merged_at_before] = merged_at_before.iso8601
attrs[:merged_after] = merged_after.iso8601
attrs[:merged_before] = merged_before.iso8601
end.compact
end
def to_default_data_attributes
{ merged_at_after: merged_at_after.iso8601, merged_at_before: merged_at_before.iso8601 }
{ merged_after: merged_after.iso8601, merged_before: merged_before.iso8601 }
end
private
......@@ -72,23 +72,23 @@ module Analytics
}
end
def validate_merged_at_after_is_earlier_than_merged_at_before
return if merged_at_after.nil? || merged_at_before.nil?
return if merged_at_after <= merged_at_before
def validate_merged_after_is_earlier_than_merged_before
return if merged_after.nil? || merged_before.nil?
return if merged_after <= merged_before
errors.add(:merged_at_before, s_('ProductivityAnalytics|is earlier than the given merged at after date'))
errors.add(:merged_before, s_('ProductivityAnalytics|is earlier than the given merged at after date'))
end
def validate_merged_at_before
return unless merged_at_before
def validate_merged_before
return unless merged_before
validate_against_productivity_analytics_start_date(:merged_at_before, merged_at_before)
validate_against_productivity_analytics_start_date(:merged_before, merged_before)
end
def validate_merged_at_after
return unless merged_at_after
def validate_merged_after
return unless merged_after
validate_against_productivity_analytics_start_date(:merged_at_after, merged_at_after)
validate_against_productivity_analytics_start_date(:merged_after, merged_after)
end
def validate_against_productivity_analytics_start_date(attribute_name, value)
......@@ -102,8 +102,8 @@ module Analytics
@productivity_analytics_start_date ||= ApplicationSetting.current&.productivity_analytics_start_date&.beginning_of_day
end
# Providing default value for `merged_at_after` and prevent setting the value to a datetime where we don't have data (`productivity_analytics_start_date`).
def default_merged_at_after
# Providing default value for `merged_after` and prevent setting the value to a datetime where we don't have data (`productivity_analytics_start_date`).
def default_merged_after
default_value = DEFAULT_DATE_RANGE.ago.to_time.utc.beginning_of_day
if productivity_analytics_start_date && productivity_analytics_start_date > default_value
......
......@@ -89,7 +89,7 @@ describe Analytics::ProductivityAnalyticsController do
end
context 'when invalid params are given' do
let(:params) { { group_id: group, merged_at_before: 10.days.ago, merged_at_after: 5.days.ago } }
let(:params) { { group_id: group, merged_before: 10.days.ago, merged_after: 5.days.ago } }
before do
group.add_owner(current_user)
......
......@@ -12,8 +12,8 @@ describe 'ProductivityAnalytics' do
author_username: 'user',
label_name: %w[label1 label2],
milestone_title: 'user',
merged_at_after: Date.yesterday.to_time,
merged_at_before: Date.today.to_time,
merged_after: Date.yesterday.to_time,
merged_before: Date.today.to_time,
group_id: group,
project_id: project.full_path
}
......@@ -46,13 +46,13 @@ describe 'ProductivityAnalytics' do
expect(element['data-label-name']).to eq(params[:label_name].join(','))
expect(element['data-milestone-title']).to eq(params[:milestone_title])
expect(element['data-merged-at-after']).to eq(params[:merged_at_after].utc.iso8601)
expect(element['data-merged-at-before']).to eq(params[:merged_at_before].utc.iso8601)
expect(element['data-merged-after']).to eq(params[:merged_after].utc.iso8601)
expect(element['data-merged-before']).to eq(params[:merged_before].utc.iso8601)
end
context 'when params are invalid' do
before do
params[:merged_at_before] = params[:merged_at_after] - 5.days # invalid
params[:merged_before] = params[:merged_after] - 5.days # invalid
end
it 'does not expose params in data attributes' do
......@@ -64,8 +64,8 @@ describe 'ProductivityAnalytics' do
expect(element['data-group-id']).to be_nil
expect(element['data-author-username']).to be_nil
expect(element['data-merged-at-before']).not_to be_nil
expect(element['data-merged-at-after']).not_to be_nil
expect(element['data-merged-before']).not_to be_nil
expect(element['data-merged-after']).not_to be_nil
end
end
end
......@@ -17,7 +17,7 @@ describe ProductivityAnalyticsFinder do
describe '.scalar_params' do
subject { described_class.scalar_params }
it { is_expected.to include(:merged_at_before, :merged_at_after) }
it { is_expected.to include(:merged_before, :merged_after) }
end
describe '#execute' do
......@@ -54,10 +54,10 @@ describe ProductivityAnalyticsFinder do
Timecop.freeze { example.run }
end
context 'with merged_at_after specified as timestamp' do
context 'with merged_after specified as timestamp' do
let(:search_params) do
{
merged_at_after: 25.days.ago.to_s
merged_after: 25.days.ago.to_s
}
end
......@@ -68,11 +68,11 @@ describe ProductivityAnalyticsFinder do
end
end
context 'with merged_at_after and merged_at_before specified' do
context 'with merged_after and merged_before specified' do
let(:search_params) do
{
merged_at_after: 30.days.ago.to_s,
merged_at_before: 20.days.ago.to_s
merged_after: 30.days.ago.to_s,
merged_before: 20.days.ago.to_s
}
end
......@@ -83,9 +83,9 @@ describe ProductivityAnalyticsFinder do
end
end
context 'with merged_at_after earlier than PA start date' do
context 'with merged_after earlier than PA start date' do
let(:search_params) do
{ merged_at_after: 3.years.ago.to_s }
{ merged_after: 3.years.ago.to_s }
end
it 'uses start_date as filter value' do
......
......@@ -87,8 +87,8 @@ describe('ProductivityApp component', () => {
wrapper.vm.$store.dispatch('filters/setInitialData', {
skipFetch: true,
data: {
mergedAtAfter: new Date('2019-09-01'),
mergedAtBefore: new Date('2019-09-02'),
mergedAfter: new Date('2019-09-01'),
mergedBefore: new Date('2019-09-02'),
},
});
wrapper.vm.$store.dispatch('filters/setGroupNamespace', 'gitlab-org');
......
......@@ -16,8 +16,8 @@ describe('Productivity analytics filter actions', () => {
const groupNamespace = 'gitlab-org';
const projectPath = 'gitlab-org/gitlab-test';
const initialData = {
mergedAtAfter: new Date('2019-11-01'),
mergedAtBefore: new Date('2019-12-09'),
mergedAfter: new Date('2019-11-01'),
mergedBefore: new Date('2019-12-09'),
minDate: new Date('2019-01-01'),
};
......@@ -244,13 +244,13 @@ describe('Productivity analytics filter actions', () => {
.catch(done.fail);
});
it('calls setUrlParams with the merged_at_after=startDate and merged_at_before=endDate', done => {
it('calls setUrlParams with the merged_after=startDate and merged_before=endDate', done => {
actions
.setDateRange(store, { startDate, endDate })
.then(() => {
expect(setUrlParams).toHaveBeenCalledWith({
merged_at_after: '2019-09-01T00:00:00Z',
merged_at_before: '2019-09-07T23:59:59Z',
merged_after: '2019-09-01T00:00:00Z',
merged_before: '2019-09-07T23:59:59Z',
});
expect(historyPushState).toHaveBeenCalled();
......
......@@ -37,8 +37,8 @@ describe('Productivity analytics filter getters', () => {
author_username: 'root',
group_id: 'gitlab-org',
label_name: ['labelxyz'],
merged_at_after: '2019-09-01T00:00:00Z',
merged_at_before: '2019-09-07T23:59:59Z',
merged_after: '2019-09-01T00:00:00Z',
merged_before: '2019-09-07T23:59:59Z',
milestone_title: 'foo',
project_id: 'gitlab-org/gitlab-test',
};
......@@ -51,20 +51,20 @@ describe('Productivity analytics filter getters', () => {
*/
describe('when chart is scatterplot', () => {
it('returns an object with common filter params and subtracts 30 days from the merged_at_after date', () => {
const mergedAtAfter = '2019-08-02';
it('returns an object with common filter params and subtracts 30 days from the merged_after date', () => {
const mergedAfter = '2019-08-02';
const expected = {
author_username: 'root',
group_id: 'gitlab-org',
label_name: ['labelxyz'],
merged_at_after: `${mergedAtAfter}T00:00:00Z`,
merged_at_before: '2019-09-07T23:59:59Z',
merged_after: `${mergedAfter}T00:00:00Z`,
merged_before: '2019-09-07T23:59:59Z',
milestone_title: 'foo',
project_id: 'gitlab-org/gitlab-test',
};
const mockGetters = {
scatterplotStartDate: new Date(mergedAtAfter),
scatterplotStartDate: new Date(mergedAfter),
};
const result = getters.getCommonFilterParams(state, mockGetters)(chartKeys.scatterplot);
......
......@@ -26,8 +26,8 @@ describe('Productivity analytics filter mutations', () => {
authorUsername,
labelName,
milestoneTitle,
mergedAtAfter: startDate,
mergedAtBefore: endDate,
mergedAfter: startDate,
mergedBefore: endDate,
minDate,
};
mutations[types.SET_INITIAL_DATA](state, initialData);
......
......@@ -8,8 +8,8 @@ describe Analytics::ProductivityAnalyticsRequestParams do
author_username: 'user',
label_name: %w[label1 label2],
milestone_title: 'user',
merged_at_after: 5.days.ago.to_time,
merged_at_before: Date.today.to_time,
merged_after: 5.days.ago.to_time,
merged_before: Date.today.to_time,
group: Group.new
}
end
......@@ -22,21 +22,21 @@ describe Analytics::ProductivityAnalyticsRequestParams do
end
describe '`merged_at` params' do
context 'when `merged_at_before` is earlier than `merged_at_after`' do
context 'when `merged_before` is earlier than `merged_after`' do
before do
params[:merged_at_after] = Date.today.to_time
params[:merged_at_before] = 5.days.ago.to_time
params[:merged_after] = Date.today.to_time
params[:merged_before] = 5.days.ago.to_time
end
it 'is invalid' do
expect(subject).to be_invalid
expect(subject.errors.messages[:merged_at_before]).not_to be_empty
expect(subject.errors.messages[:merged_before]).not_to be_empty
end
end
context 'when `merged_at_after` is earlier than `productivity_analytics_start_date`' do
context 'when `merged_after` is earlier than `productivity_analytics_start_date`' do
before do
params[:merged_at_after] = 5.days.ago.to_time
params[:merged_after] = 5.days.ago.to_time
allow(ApplicationSetting)
.to receive(:current)
......@@ -45,13 +45,13 @@ describe Analytics::ProductivityAnalyticsRequestParams do
it 'is invalid' do
expect(subject).to be_invalid
expect(subject.errors.messages[:merged_at_after]).not_to be_empty
expect(subject.errors.messages[:merged_after]).not_to be_empty
end
end
context 'when `merged_at_before` is earlier than `productivity_analytics_start_date`' do
context 'when `merged_before` is earlier than `productivity_analytics_start_date`' do
before do
params[:merged_at_before] = 5.days.ago.to_time
params[:merged_before] = 5.days.ago.to_time
allow(ApplicationSetting)
.to receive(:current)
......@@ -60,7 +60,7 @@ describe Analytics::ProductivityAnalyticsRequestParams do
it 'is invalid' do
expect(subject).to be_invalid
expect(subject.errors.messages[:merged_at_before]).not_to be_empty
expect(subject.errors.messages[:merged_before]).not_to be_empty
end
end
end
......@@ -71,13 +71,13 @@ describe Analytics::ProductivityAnalyticsRequestParams do
Timecop.freeze { example.run }
end
describe '`merged_at_before`' do
describe '`merged_before`' do
it 'defaults to today date' do
expect(described_class.new.merged_at_before).to eq(Date.today.at_end_of_day)
expect(described_class.new.merged_before).to eq(Date.today.at_end_of_day)
end
end
describe '`merged_at_after`' do
describe '`merged_after`' do
context 'when `productivity_analytics_start_date` is within the last 30 days' do
before do
allow(ApplicationSetting)
......@@ -86,7 +86,7 @@ describe Analytics::ProductivityAnalyticsRequestParams do
end
it 'defaults to `productivity_analytics_start_date`' do
expect(described_class.new.merged_at_after).to eq(ApplicationSetting.current.productivity_analytics_start_date.beginning_of_day)
expect(described_class.new.merged_after).to eq(ApplicationSetting.current.productivity_analytics_start_date.beginning_of_day)
end
end
......@@ -98,7 +98,7 @@ describe Analytics::ProductivityAnalyticsRequestParams do
end
it 'defaults to 30 days ago' do
expect(described_class.new.merged_at_after).to eq(30.days.ago.to_time.utc.beginning_of_day)
expect(described_class.new.merged_after).to eq(30.days.ago.to_time.utc.beginning_of_day)
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment