Skip to content

feat: standardize frontend segments calls (CM-189) #4013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ulemons merged 2 commits into from
Apr 17, 2026
Merged

feat: standardize frontend segments calls (CM-189) #4013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9645a3
feat: add fe fix for standardize
ulemons Apr 10, 2026
a5dcec9
fix: lint
ulemons Apr 13, 2026
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
17 changes: 11 additions & 6 deletions backend/src/middlewares/segmentMiddleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ export async function segmentMiddleware(req, res, next) {
// read req.params directly and ignore req.currentSegments entirely — so the
// resolution below is harmless for those endpoints.
if (req.query.segments) {
// GET requests
// GET requests — req.query values can be string or string[], normalize to array
const segmentIds = ([] as string[]).concat(req.query.segments)
segments = {
rows: await resolveToLeafSegments(segmentRepository, req.query.segments, req),
rows: await resolveToLeafSegments(segmentRepository, segmentIds, req),
}
} else if (req.body.segments) {
// POST/PUT requests
// POST/PUT requests — body.segments should always be an array, but normalize defensively
const segmentIds = ([] as string[]).concat(req.body.segments)
segments = {
rows: await resolveToLeafSegments(segmentRepository, req.body.segments, req),
rows: await resolveToLeafSegments(segmentRepository, segmentIds, req),
}
} else {
segments = await segmentRepository.querySubprojects({ limit: 1, offset: 0 })
Expand Down Expand Up @@ -60,8 +62,11 @@ async function resolveToLeafSegments(

const nonLeaf = fetched.filter((s) => !isSegmentSubproject(s))

const segmentLevel = (s: any) =>
s.grandparentSlug ? 'subproject' : s.parentSlug ? 'project' : 'projectGroup'
const segmentLevel = (s: any) => {
if (s.grandparentSlug) return 'subproject'
if (s.parentSlug) return 'project'
return 'projectGroup'
}

if (nonLeaf.length === 0) {
// All IDs are already leaf segments — current behavior, no change.
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/modules/activity/components/activity-timeline.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ const fetchActivities = async ({ reset } = { reset: false }) => {
offset: offset.value,
segments: selectedSegment.value
? [selectedSegment.value]
: segments.value.map((s) => s.id),
: [selectedProjectGroup.value.id],
Copy link
Copy Markdown

@cursor cursor bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check on selectedProjectGroup causes runtime crash

High Severity

selectedProjectGroup.value.id lacks a null/undefined guard. The previous code used segments.value.map((s) => s.id) which was backed by a computed that gracefully returned [] when selectedProjectGroup.value was null (via getSegmentsFromProjectGroup's null check). The new code directly dereferences .id without optional chaining, causing a TypeError if no project group is selected. Every other file in this same PR applies a null check (e.g. ?.id, if (projectGroup), ternary guard).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a5dcec9. Configure here.

});
Comment on lines 381 to 384
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectedProjectGroup.value.id can throw when selectedProjectGroup.value is still null/undefined (e.g., initial mount before the segments store resolves). Consider guarding this path (return early until selectedProjectGroup.value?.id exists) and/or sending segments: null when no project group is selected, consistent with other services, rather than assuming it’s always present.

Copilot uses AI. Check for mistakes.

loading.value = false;
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/modules/lf/layout/components/lf-banners.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ import {
watch, ref, computed, onUnmounted,
} from 'vue';
import { IntegrationService } from '@/modules/integration/integration-service';
import { getSegmentsFromProjectGroup } from '@/utils/segments';
import { isCurrentDateAfterGivenWorkingDays } from '@/utils/date';
import { useRoute } from 'vue-router';
import usePermissions from '@/shared/modules/permissions/helpers/usePermissions';
Expand Down Expand Up @@ -227,7 +226,7 @@ const showBanner = computed(() => (integrationsWithErrors.value.length

const fetchIntegrations = (projectGroup) => {
if (projectGroup) {
IntegrationService.list(null, null, null, null, getSegmentsFromProjectGroup(projectGroup))
IntegrationService.list(null, null, null, null, [projectGroup.id])
.then((response) => {
integrations.value = response.rows;
})
Expand Down
8 changes: 3 additions & 5 deletions frontend/src/modules/organization/organization-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import authAxios from '@/shared/axios/auth-axios';
import { AuthService } from '@/modules/auth/services/auth.service'; import { storeToRefs } from 'pinia';
import { useLfSegmentsStore } from '@/modules/lf/segments/store';
import { getSegmentsFromProjectGroup } from '@/utils/segments';

const getSelectedProjectGroup = () => {
const lsSegmentsStore = useLfSegmentsStore();
Expand Down Expand Up @@ -216,10 +215,9 @@ export class OrganizationService {
}

static async fetchMergeSuggestions(limit, offset, query) {
const segments = [
...getSegmentsFromProjectGroup(getSelectedProjectGroup()),
getSelectedProjectGroup().id,
];
const segments = getSelectedProjectGroup()?.id
? [getSelectedProjectGroup().id]
: [];
Comment on lines +218 to +220
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This computes segments as [] when there is no selected project group. As with other calls in this service (e.g., update/create/destroyAll), sending null/omitting segments is safer than [] because the backend segment middleware treats an empty array as “segments provided” and won’t apply defaults. Also, getSelectedProjectGroup() is invoked multiple times; capturing it once avoids a possible null-deref if the store changes between the condition and the true branch.

Copilot uses AI. Check for mistakes.

const data = {
limit,
Expand Down
9 changes: 1 addition & 8 deletions frontend/src/modules/organization/services/organization.api.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import authAxios from '@/shared/axios/auth-axios';
import { Organization } from '@/modules/organization/types/Organization';
import { getSegmentsFromProjectGroup } from '@/utils/segments';
import { useLfSegmentsStore } from '@/modules/lf/segments/store';
import { storeToRefs } from 'pinia';

Expand Down Expand Up @@ -30,13 +29,7 @@ export class OrganizationApiService {
}

static async fetchMergeSuggestions(limit: number = 20, offset: number = 0, query: any = {}) {
const lsSegmentsStore = useLfSegmentsStore();
const { selectedProjectGroup } = storeToRefs(lsSegmentsStore);

const segments = [
...getSegmentsFromProjectGroup(selectedProjectGroup.value),
selectedProjectGroup.value?.id,
];
const segments = getSelectedProjectGroupId() ?? [];
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchMergeSuggestions forces segments to [] when no project group is selected (getSelectedProjectGroupId() returns null). Sending an empty array changes backend behavior: segmentMiddleware treats body.segments as present/truthy and sets req.currentSegments to an empty list instead of applying its default segment selection. Prefer sending null/omitting segments when no project group is selected (e.g., use the helper return value directly without ?? []).

Suggested change
const segments = getSelectedProjectGroupId() ?? [];
const segments = getSelectedProjectGroupId();

Copilot uses AI. Check for mistakes.

const data = {
limit,
Expand Down
Loading