Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ export function getTrafficAnalysisQueryPlaceholdersFilled({
}

if (numTemporalSeries > 1 && week && week > 0) {
dimensions.push('week');
dimensions.push('year', 'week');
} else if (numTemporalSeries > 1 && month && month > 0) {
dimensions.push('month');
dimensions.push('year', 'month');
}
const dimensionColumns = dimensions.join(', ');
const dimensionColumnsPrefixed = dimensions.map((col) => `a.${col}`).join(', ');
Expand Down Expand Up @@ -179,6 +179,7 @@ WITH min_totals AS (
),
raw AS (
SELECT
year,
week,
month,
path,
Expand Down Expand Up @@ -279,6 +280,7 @@ WITH min_totals AS (
),
raw AS (
SELECT
year,
week,
month,
path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ WITH min_totals AS (
),
raw AS (
SELECT
year,
week,
month,
path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const TrafficDataResponseDto = {
* Converts a traffic data object into a JSON object.
* @param {object} data - traffic data object.
* @returns {{
* year: number,
* type: string,
* channel: string,
* platform: string,
Expand All @@ -33,6 +34,7 @@ export const TrafficDataResponseDto = {
* }} JSON object.
*/
toJSON: (data) => ({
year: data.year,
week: data.week,
month: data.month,
type: data.trf_type,
Expand Down
40 changes: 21 additions & 19 deletions packages/spacecat-shared-athena-client/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -824,9 +824,9 @@ describe('Traffic analysis query functions', () => {

const result = getTrafficAnalysisQueryPlaceholdersFilled(params);

expect(result.dimensionColumns).to.equal('device, week');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.week');
expect(result.groupBy).to.equal('device, week');
expect(result.dimensionColumns).to.equal('device, year, week');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.year, a.week');
expect(result.groupBy).to.equal('device, year, week');
});

it('should add month to dimensions when numTemporalSeries > 1 and month is provided', () => {
Expand All @@ -841,9 +841,9 @@ describe('Traffic analysis query functions', () => {

const result = getTrafficAnalysisQueryPlaceholdersFilled(params);

expect(result.dimensionColumns).to.equal('device, month');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.month');
expect(result.groupBy).to.equal('device, month');
expect(result.dimensionColumns).to.equal('device, year, month');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.year, a.month');
expect(result.groupBy).to.equal('device, year, month');
});

it('should prefer week over month when both are provided with numTemporalSeries > 1', () => {
Expand All @@ -859,9 +859,9 @@ describe('Traffic analysis query functions', () => {

const result = getTrafficAnalysisQueryPlaceholdersFilled(params);

expect(result.dimensionColumns).to.equal('device, week');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.week');
expect(result.groupBy).to.equal('device, week');
expect(result.dimensionColumns).to.equal('device, year, week');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.year, a.week');
expect(result.groupBy).to.equal('device, year, week');
expect(result.dimensionColumns).to.not.include('month');
});

Expand Down Expand Up @@ -927,9 +927,9 @@ describe('Traffic analysis query functions', () => {

const result = getTrafficAnalysisQueryPlaceholdersFilled(params);

expect(result.dimensionColumns).to.equal('device, utm_campaign, week');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.utm_campaign, a.week');
expect(result.groupBy).to.equal('device, utm_campaign, week');
expect(result.dimensionColumns).to.equal('device, utm_campaign, year, week');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.utm_campaign, a.year, a.week');
expect(result.groupBy).to.equal('device, utm_campaign, year, week');
});

it('should handle multiple dimensions with month when numTemporalSeries > 1', () => {
Expand All @@ -944,9 +944,9 @@ describe('Traffic analysis query functions', () => {

const result = getTrafficAnalysisQueryPlaceholdersFilled(params);

expect(result.dimensionColumns).to.equal('device, utm_campaign, month');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.utm_campaign, a.month');
expect(result.groupBy).to.equal('device, utm_campaign, month');
expect(result.dimensionColumns).to.equal('device, utm_campaign, year, month');
expect(result.dimensionColumnsPrefixed).to.equal('a.device, a.utm_campaign, a.year, a.month');
expect(result.groupBy).to.equal('device, utm_campaign, year, month');
});

it('should not add temporal dimension when neither week nor month is provided', () => {
Expand Down Expand Up @@ -979,8 +979,8 @@ describe('Traffic analysis query functions', () => {
const placeholders = getTrafficAnalysisQueryPlaceholdersFilled(params);
const sql = getTrafficAnalysisQuery(placeholders);

expect(sql).to.include('device, week');
expect(sql).to.include('a.device, a.week');
expect(sql).to.include('device, year, week');
expect(sql).to.include('a.device, a.year, a.week');
expect(sql).to.include('week=28');

// Verify no dangling commas
Expand All @@ -1001,8 +1001,8 @@ describe('Traffic analysis query functions', () => {
const placeholders = getTrafficAnalysisQueryPlaceholdersFilled(params);
const sql = getTrafficAnalysisQuery(placeholders);

expect(sql).to.include('device, month');
expect(sql).to.include('a.device, a.month');
expect(sql).to.include('device, year, month');
expect(sql).to.include('a.device, a.year, a.month');
expect(sql).to.include('month=7');

// Verify no dangling commas
Expand All @@ -1017,6 +1017,7 @@ describe('DTO Tests', () => {
describe('TrafficDataResponseDto', () => {
it('should convert traffic data to JSON format', () => {
const inputData = {
year: 2025,
week: 12,
month: 6,
trf_type: 'organic',
Expand All @@ -1039,6 +1040,7 @@ describe('DTO Tests', () => {
const result = TrafficDataResponseDto.toJSON(inputData);

expect(result).to.deep.equal({
year: 2025,
week: 12,
month: 6,
type: 'organic',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,7 @@ describe('Traffic Analysis Queries', () => {

// Verify all columns in raw CTE
const rawColumns = [
'year',
'week',
'month',
'path',
Expand Down Expand Up @@ -1041,6 +1042,7 @@ describe('Traffic Analysis Queries', () => {

// Verify all columns in raw CTE
const rawColumns = [
'year',
'week',
'month',
'path',
Expand Down
4 changes: 2 additions & 2 deletions packages/spacecat-shared-utils/src/calendar-week-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,15 +225,15 @@ export function getTemporalCondition({
let currentYear = year;
if (currentWeek < 1) {
currentYear -= 1;
currentWeek = has53CalendarWeeks(currentYear) ? 53 : 52;
currentWeek += has53CalendarWeeks(currentYear) ? 53 : 52;
}
log?.info(`[getTemporalCondition] currentWeek: ${currentWeek}, currentYear: ${currentYear}`);
conditions.push(getWeekInfo(currentWeek, currentYear).temporalCondition);
} else if (hasValidMonth) {
let currentMonth = month - i;
let currentYear = year;
if (currentMonth < 1) {
currentMonth = 12;
currentMonth += 12;
currentYear -= 1;
}
log?.info(`[getTemporalCondition] currentMonth: ${currentMonth}, currentYear: ${currentYear}`);
Expand Down
40 changes: 20 additions & 20 deletions packages/spacecat-shared-utils/test/calendar-week-helper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,30 +542,22 @@ describe('Utils - temporal helpers', () => {
expect(c).to.equal('(year=2025 AND month=2) OR (year=2025 AND month=1) OR (year=2024 AND month=12)');
});

it('handles multiple transitions when week goes below 1 (demonstrates current implementation)', () => {
// Week 1 of 2025, going back 5 weeks
// Note: current implementation has a bug where week becomes 0, -1, -2, etc.
// and always gets set to 52 (since 2025 is not a 53-week year)
// This test documents the actual behavior
it('correctly wraps weeks across year boundary', () => {
// Week 1 of 2025, going back 5 weeks → weeks 1, 52, 51, 50, 49
const c = getTemporalCondition({ week: 1, year: 2025, numSeries: 5 });
const parts = c.split(' OR ');
expect(parts.length).to.be.at.least(5);
expect(c).to.include('(year=2025 AND month=1 AND week=1)');
// All subsequent weeks will be set to week 52 due to the implementation
expect(c).to.include('(year=2024 AND month=12 AND week=52)');
expect(c).to.include('(year=2024 AND month=12 AND week=51)');
expect(c).to.include('(year=2024 AND month=12 AND week=50)');
expect(c).to.include('(year=2024 AND month=12 AND week=49)');
});

it('handles multiple transitions when month goes below 1 (demonstrates current implementation)', () => {
// Month 1 of 2025, going back 5 months
// Note: current implementation has a bug where month becomes 0, -1, -2, etc.
// and always gets set to 12
// This test documents the actual behavior
it('correctly wraps months across year boundary', () => {
// Month 1 of 2025, going back 5 months → months 1, 12, 11, 10, 9
const c = getTemporalCondition({ month: 1, year: 2025, numSeries: 5 });
const parts = c.split(' OR ');
expect(parts.length).to.equal(5);
expect(c).to.include('(year=2025 AND month=1)');
// All subsequent months will be set to month 12 due to the implementation
expect(c).to.include('(year=2024 AND month=12)');
expect(c).to.equal(
'(year=2025 AND month=1) OR (year=2024 AND month=12) OR (year=2024 AND month=11) OR (year=2024 AND month=10) OR (year=2024 AND month=9)',
);
});

it('prefers week over month when both are provided with numSeries > 1', () => {
Expand Down Expand Up @@ -597,8 +589,16 @@ describe('Utils - temporal helpers', () => {
expect(parts.length).to.equal(15);
expect(parts[0]).to.equal('(year=2025 AND month=12)');
expect(parts[11]).to.equal('(year=2025 AND month=1)');
// After month goes below 1, it becomes 12 repeatedly due to implementation
expect(parts[14]).to.equal('(year=2024 AND month=12)');
expect(parts[12]).to.equal('(year=2024 AND month=12)');
expect(parts[13]).to.equal('(year=2024 AND month=11)');
expect(parts[14]).to.equal('(year=2024 AND month=10)');
});

it('handles month=12 with numSeries=4 (no year wrap)', () => {
const c = getTemporalCondition({ month: 12, year: 2025, numSeries: 4 });
expect(c).to.equal(
'(year=2025 AND month=12) OR (year=2025 AND month=11) OR (year=2025 AND month=10) OR (year=2025 AND month=9)',
);
});
});
});
Expand Down