Skip to content

Remove non-standard types and align code with schema protocol #286

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
hotlong merged 3 commits into from
Jan 27, 2026
Merged

Remove non-standard types and align code with schema protocol #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86395e0
Initial plan
Copilot Jan 27, 2026
adfd9d4
fix: add missing donut chart type and geolocation field type to schemas
Copilot Jan 27, 2026
8792d00
refactor: remove non-standard types and align code with schema protocol
Copilot Jan 27, 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
2 changes: 1 addition & 1 deletion examples/crm/src/ui/dashboards.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export const ServiceDashboard: Dashboard = {
// Row 2: Case Distribution
{
title: 'Cases by Status',
type: 'donut',
type: 'pie',
object: 'case',
filter: { is_closed: false },
categoryField: 'status',
Expand Down
19 changes: 1 addition & 18 deletions packages/spec/src/data/field.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('FieldType', () => {
'lookup', 'master_detail',
'image', 'file', 'avatar',
'formula', 'summary', 'autonumber',
'location', 'geolocation', 'address', 'code', 'color', 'rating', 'slider', 'signature', 'qrcode'
'location', 'address', 'code', 'color', 'rating', 'slider', 'signature', 'qrcode'
];

validTypes.forEach(type => {
Expand Down Expand Up @@ -495,10 +495,6 @@ describe('Field Factory Helpers', () => {
expect(() => FieldType.parse('location')).not.toThrow();
});

it('should accept geolocation field type', () => {
expect(() => FieldType.parse('geolocation')).not.toThrow();
});

it('should accept address field type', () => {
expect(() => FieldType.parse('address')).not.toThrow();
});
Expand Down Expand Up @@ -647,19 +643,6 @@ describe('Field Factory Helpers', () => {
expect(qrcodeField.displayValue).toBe(true);
expect(qrcodeField.allowScanning).toBe(true);
});

it('should create geolocation field', () => {
const geolocationField = Field.geolocation({
label: 'Current Location',
displayMap: true,
allowGeocoding: false,
});

expect(geolocationField.type).toBe('geolocation');
expect(geolocationField.label).toBe('Current Location');
expect(geolocationField.displayMap).toBe(true);
expect(geolocationField.allowGeocoding).toBe(false);
});
});

describe('Currency Field Type with Multi-Currency Support', () => {
Expand Down
5 changes: 0 additions & 5 deletions packages/spec/src/data/field.zod.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,4 @@ export const Field = {
type: 'qrcode',
...config
} as const),

geolocation: (config: FieldInput = {}) => ({
type: 'geolocation',
...config
} as const),
};
6 changes: 3 additions & 3 deletions packages/spec/src/ui/dashboard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import {

describe('ChartType', () => {
it('should accept all chart types', () => {
const types = ['metric', 'bar', 'line', 'pie', 'donut', 'funnel', 'table', 'text'];
const types = ['metric', 'bar', 'line', 'pie', 'funnel', 'table', 'text'];

Comment on lines +13 to 14

Copilot AI Jan 27, 2026

Copy link

Choose a reason for hiding this comment

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

The test name says it "should accept all chart types", but this list omits several valid ChartType values defined in dashboard.zod.ts (e.g. radar, scatter, heatmap, pivot, list, image, frame). Consider either deriving the list from the enum (so it stays in sync) or renaming the test to reflect that it only checks a subset.

Suggested change
const types = ['metric', 'bar', 'line', 'pie', 'funnel', 'table', 'text'];
const types: string[] = (ChartType as any).options ?? [];

Copilot uses AI. Check for mistakes.
types.forEach(type => {
expect(() => ChartType.parse(type)).not.toThrow();
});
});

it('should reject invalid chart types', () => {
expect(() => ChartType.parse('scatter')).toThrow();
expect(() => ChartType.parse('bubble')).toThrow();
expect(() => ChartType.parse('invalid')).toThrow();
Comment on lines 20 to 22

Copilot AI Jan 27, 2026

Copy link

Choose a reason for hiding this comment

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

Since this PR removes support for donut, it would be useful to explicitly assert that ChartType.parse('donut') throws in the "reject invalid chart types" test. Right now the test only checks bubble/invalid, so a future reintroduction of donut wouldn’t be caught.

Copilot uses AI. Check for mistakes.
});
});
Expand Down Expand Up @@ -255,7 +255,7 @@ describe('DashboardSchema', () => {
},
{
title: 'Opportunities by Type',
type: 'donut',
type: 'pie',
object: 'opportunity',
categoryField: 'type',
aggregate: 'count',
Expand Down
Loading