Skip to content

Commit 83ddfc3

Browse files
refactor(OpenAPISyncTab): streamline component logic and enhance user feedback (#7483)
- Removed unused props and improved error handling in OpenAPISyncTab components. - Updated messaging in CollectionStatusSection and OverviewSection for clarity. - Enhanced the SpecDiffModal to provide better visual feedback on changes. - Refactored sync flow logic to ensure accurate endpoint categorization and improved performance. - Added new utility functions for better handling of spec changes and endpoint comparisons.
1 parent 1ab296f commit 83ddfc3

File tree

9 files changed

+275
-288
lines changed

9 files changed

+275
-288
lines changed

packages/bruno-app/src/components/OpenAPISyncTab/CollectionStatusSection/index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ const CollectionStatusSection = ({
7171
variant: 'muted',
7272
message: 'Collection has changes since last sync',
7373
badges: { modifiedCount, missingCount, localOnlyCount },
74+
version,
75+
lastSyncDate,
7476
actions: ['revert-all']
7577
};
7678
}
@@ -244,7 +246,7 @@ const CollectionStatusSection = ({
244246
<div className="sync-review-empty-state mt-5">
245247
<IconCheck size={40} className="empty-state-icon" />
246248
<h4>No changes in collection</h4>
247-
<p>The collection matches the last synced spec. Nothing to review.</p>
249+
<p>The collection endpoints match the last synced spec. Nothing to review.</p>
248250
</div>
249251
)}
250252
{/* Action confirmation modal */}

packages/bruno-app/src/components/OpenAPISyncTab/OverviewSection/index.js

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { countEndpoints } from '../utils';
66
import moment from 'moment';
77
import { IconCheck } from '@tabler/icons';
88
import Button from 'ui/Button';
9-
import StatusBadge from 'ui/StatusBadge';
109
import Help from 'components/Help';
1110

1211
const capitalize = (str) => str ? str.charAt(0).toUpperCase() + str.slice(1) : str;
@@ -40,14 +39,16 @@ const SUMMARY_CARDS = [
4039
}
4140
];
4241

43-
const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, remoteDrift, onTabSelect, error, fileNotFound, onOpenSettings }) => {
42+
const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, remoteDrift, onTabSelect, error, onOpenSettings }) => {
4443
const openApiSyncConfig = collection?.brunoConfig?.openapi?.[0];
4544

4645
const reduxError = useSelector((state) => state.openapiSync?.collectionUpdates?.[collection.uid]?.error);
4746
const specMeta = useSelector(selectStoredSpecMeta(collection.uid));
4847
const activeError = error || reduxError;
4948

5049
const version = storedSpec?.info?.version ?? specMeta?.version;
50+
const newVersion = specDrift?.newVersion;
51+
const hasVersionChange = version && newVersion && version !== newVersion;
5152
const endpointCount = countEndpoints(storedSpec) ?? specMeta?.endpointCount ?? null;
5253
const lastSyncDate = openApiSyncConfig?.lastSyncDate;
5354
const groupBy = openApiSyncConfig?.groupBy || 'tags';
@@ -89,7 +90,7 @@ const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, r
8990
};
9091

9192
const details = [
92-
{ label: 'Spec Version', value: version ? `v${version}` : '–' },
93+
{ label: 'Spec Version', value: hasVersionChange ? `v${version} → v${newVersion}` : version ? `v${version}` : '–' },
9394
{ label: 'Endpoints in Spec', value: endpointCount != null ? endpointCount : '–' },
9495
{ label: 'Last Synced At', value: lastSyncDate ? moment(lastSyncDate).fromNow() : '–', tooltip: lastSyncDate ? moment(lastSyncDate).format('MMMM D, YYYY [at] h:mm A') : undefined },
9596
{ label: 'Folder Grouping', value: capitalize(groupBy) },
@@ -100,6 +101,10 @@ const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, r
100101
const hasSpecUpdates = specUpdatesPending > 0;
101102

102103
const bannerState = useMemo(() => {
104+
const versionInfo = (specDrift?.storedVersion && specDrift?.newVersion && specDrift.storedVersion !== specDrift.newVersion)
105+
? ` (v${specDrift.storedVersion} → v${specDrift.newVersion})`
106+
: '';
107+
103108
if (activeError) {
104109
return {
105110
variant: 'danger',
@@ -128,15 +133,15 @@ const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, r
128133
if (hasSpecUpdates && hasCollectionChanges) {
129134
return {
130135
variant: 'warning',
131-
title: 'The API spec has new updates and the collection has changes',
136+
title: `The API spec has new updates${versionInfo} and the collection has changes`,
132137
subtitle: 'New or changed requests are available. Some collection changes may be overwritten.',
133138
buttons: ['sync', 'changes']
134139
};
135140
}
136141
if (hasSpecUpdates) {
137142
return {
138143
variant: 'warning',
139-
title: 'The API spec has new updates',
144+
title: `The API spec has new updates${versionInfo}`,
140145
subtitle: 'New or changed requests are available.',
141146
buttons: ['sync']
142147
};
@@ -156,7 +161,7 @@ const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, r
156161
// buttons: []
157162
// };
158163
return null;
159-
}, [activeError, fileNotFound, hasDriftData, hasSpecUpdates, hasCollectionChanges, specDrift?.storedSpecMissing, lastSyncDate]);
164+
}, [activeError, hasDriftData, hasSpecUpdates, hasCollectionChanges, specDrift?.storedSpecMissing, specDrift?.storedVersion, specDrift?.newVersion, lastSyncDate]);
160165

161166
return (
162167
<div className="overview-section">
@@ -168,12 +173,6 @@ const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, r
168173
? <IconCheck size={16} className="status-check-icon" />
169174
: <div className={`status-dot ${bannerState.variant}`} />}
170175
<span className="banner-title">{bannerState.title}</span>
171-
{bannerState.showBadge && (
172-
<StatusBadge status="info" radius="full">{specUpdatesPending} {specUpdatesPending === 1 ? 'spec update' : 'spec updates'}</StatusBadge>
173-
)}
174-
{bannerState.showChangesBadge && (
175-
<StatusBadge status="warning" radius="full">{changedInCollection} {changedInCollection === 1 ? 'collection change' : 'collection changes'}</StatusBadge>
176-
)}
177176
</div>
178177
{bannerState.subtitle && (
179178
<p className="banner-subtitle">{bannerState.subtitle}</p>
@@ -201,6 +200,11 @@ const OverviewSection = ({ collection, storedSpec, collectionDrift, specDrift, r
201200
Restore Spec File
202201
</Button>
203202
)}
203+
{bannerState.buttons.includes('spec-details') && (
204+
<Button variant="outline" size="sm" onClick={() => onTabSelect('spec-updates')}>
205+
View Details
206+
</Button>
207+
)}
204208
{bannerState.buttons.includes('open-settings') && (
205209
<Button variant="outline" size="sm" onClick={onOpenSettings}>
206210
Update connection settings

packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ const SpecDiffModal = ({ specDrift, onClose }) => {
4747
>
4848
<div className="spec-diff-modal">
4949
<div className="spec-diff-badges">
50+
{modifiedCount > 0 && <StatusBadge status="warning">Updated: {modifiedCount}</StatusBadge>}
5051
{addedCount > 0 && <StatusBadge status="success">Added: {addedCount}</StatusBadge>}
51-
{modifiedCount > 0 && <StatusBadge status="info">Updated: {modifiedCount}</StatusBadge>}
5252
{removedCount > 0 && <StatusBadge status="danger">Removed: {removedCount}</StatusBadge>}
5353
{versionLabel && <StatusBadge>{versionLabel}</StatusBadge>}
5454
</div>

packages/bruno-app/src/components/OpenAPISyncTab/SpecStatusSection/index.js

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import { useMemo } from 'react';
22
import { useSelector } from 'react-redux';
33
import {
44
IconCheck,
5-
IconRefresh
5+
IconRefresh,
6+
IconAlertTriangle
67
} from '@tabler/icons';
78
import Button from 'ui/Button';
89
import StatusBadge from 'ui/StatusBadge';
@@ -35,28 +36,19 @@ const SpecStatusSection = ({
3536
return { variant: 'danger', message: `Source file not found at ${sourceUrl}`, actions: ['open-settings'] };
3637
}
3738
if (error || specDrift?.isValid === false) {
38-
return { variant: 'danger', message: error || specDrift?.error || 'Invalid OpenAPI specification', actions: [] };
39+
return { variant: 'danger', message: error || specDrift?.error || 'Invalid OpenAPI specification', actions: ['open-settings'] };
3940
}
4041
if (!specDrift) {
4142
return null;
42-
// TODO: re-enable success banner
43-
// if (!lastSyncedAt) return null;
44-
// return {
45-
// variant: 'success', message: 'Spec is up to date', actions: [],
46-
// version: storedSpec?.info?.version,
47-
// lastChecked: moment(lastCheckedAt || lastSyncedAt).fromNow()
48-
// };
4943
}
5044
if (specDrift.storedSpecMissing) {
5145
if (!lastSyncedAt) {
5246
return { variant: 'warning', message: 'Initial sync required — your collection differs from the spec', actions: [] };
5347
}
54-
if (specDrift.hasRemoteChanges) {
55-
return { variant: 'warning', message: 'Last synced spec not found — Restore the latest spec from the source to track future changes.', actions: [] };
56-
}
5748
return { variant: 'warning', message: 'Last synced spec not found — Restore the latest spec from the source to track future changes.', actions: [] };
5849
}
59-
if (specDrift.hasRemoteChanges) {
50+
const hasEndpointUpdates = (specDrift.added?.length || 0) + (specDrift.modified?.length || 0) + (specDrift.removed?.length || 0) > 0;
51+
if (hasEndpointUpdates) {
6052
const versionInfo = (specDrift.storedVersion && specDrift.newVersion && specDrift.storedVersion !== specDrift.newVersion)
6153
? ` (v${specDrift.storedVersion} → v${specDrift.newVersion})`
6254
: '';
@@ -71,7 +63,7 @@ const SpecStatusSection = ({
7163
// lastChecked: lastCheckedAt ? moment(lastCheckedAt).fromNow() : 'just now'
7264
// };
7365
return null;
74-
}, [isLoading, fileNotFound, error, sourceUrl, specDrift, lastSyncedAt, storedSpec, lastCheckedAt]);
66+
}, [fileNotFound, error, sourceUrl, specDrift, lastSyncedAt, storedSpec, lastCheckedAt]);
7567
return (
7668
<>
7769
{bannerState && (
@@ -93,8 +85,8 @@ const SpecStatusSection = ({
9385
</span>
9486
{bannerState.changes && (
9587
<span className="banner-details">
88+
{bannerState.changes.modified > 0 && <StatusBadge key="modified" status="warning" radius="full">{bannerState.changes.modified} {bannerState.changes.modified > 1 ? 'endpoints' : 'endpoint'} updated</StatusBadge>}
9689
{bannerState.changes.added > 0 && <StatusBadge key="added" status="success" radius="full">{bannerState.changes.added} {bannerState.changes.added > 1 ? 'endpoints' : 'endpoint'} added</StatusBadge>}
97-
{bannerState.changes.modified > 0 && <StatusBadge key="modified" status="info" radius="full">{bannerState.changes.modified} {bannerState.changes.modified > 1 ? 'endpoints' : 'endpoint'} updated</StatusBadge>}
9890
{bannerState.changes.removed > 0 && <StatusBadge key="removed" status="danger" radius="full">{bannerState.changes.removed} {bannerState.changes.removed > 1 ? 'endpoints' : 'endpoint'} removed</StatusBadge>}
9991
</span>
10092
)}
@@ -113,7 +105,13 @@ const SpecStatusSection = ({
113105
</div>
114106
)}
115107

116-
{specDrift?.storedSpecMissing && openApiSyncConfig?.lastSyncDate ? (
108+
{(error || fileNotFound || specDrift?.isValid === false) ? (
109+
<div className="sync-review-empty-state mt-5">
110+
<IconAlertTriangle size={40} className="empty-state-icon" />
111+
<h4>Unable to check for updates</h4>
112+
<p>Fix the connection issue above and check again.</p>
113+
</div>
114+
) : specDrift?.storedSpecMissing && openApiSyncConfig?.lastSyncDate ? (
117115
<div className="sync-review-empty-state mt-5">
118116
<IconRefresh size={40} className="empty-state-icon" />
119117
<h4>Last Synced Spec not found in storage</h4>

packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,18 @@ import { setReviewDecision, setReviewDecisions, selectTabUiState } from 'provide
2828
* - specRemovedEndpoints: removed from spec, still in collection
2929
*/
3030
const categorizeEndpoints = (remoteDrift, specDrift, collectionDrift) => {
31-
const specAddedEndpoints = remoteDrift.missing || [];
32-
const specRemovedEndpoints = remoteDrift.localOnly || [];
31+
// Only show endpoints as "New in Spec" if they were actually added to the spec
32+
// (i.e., they appear in specDrift.added). Endpoints the user deleted locally that
33+
// still exist in both stored and remote spec should not appear here — they belong
34+
// in "Collection Changes" only.
35+
const specAddedIds = new Set((specDrift?.added || []).map((ep) => ep.id));
36+
const specAddedEndpoints = (remoteDrift.missing || []).filter((ep) => specAddedIds.has(ep.id));
37+
38+
// Only show endpoints as "Removed from Spec" if they were actually in the stored spec
39+
// (i.e., they appear in specDrift.removed). Locally-added endpoints that were never in
40+
// the spec should not appear here — they belong in "Collection Changes" only.
41+
const specRemovedIds = new Set((specDrift?.removed || []).map((ep) => ep.id));
42+
const specRemovedEndpoints = (remoteDrift.localOnly || []).filter((ep) => specRemovedIds.has(ep.id));
3343

3444
// Build lookup sets to determine who changed each modified endpoint
3545
const specModifiedIds = new Set((specDrift?.modified || []).map((ep) => ep.id));
@@ -154,10 +164,7 @@ const SyncReviewPage = ({
154164

155165
// Accepted — changes that will be applied
156166
addGroup('New endpoints to add', 'add', specAddedEndpoints.filter(isAccepted));
157-
addGroup('Endpoints to update', 'update', [
158-
...specUpdatedEndpoints.filter(isAccepted),
159-
...localUpdatedEndpoints.filter(isAccepted)
160-
]);
167+
addGroup('Endpoints to update', 'update', specUpdatedEndpoints.filter(isAccepted));
161168
addGroup('Endpoints to delete', 'remove', specRemovedEndpoints.filter(isAccepted));
162169

163170
// Skipped — changes that will be preserved as-is
@@ -167,7 +174,7 @@ const SyncReviewPage = ({
167174
addGroup('Keeping current version (skipped updates)', 'keep', specUpdatedEndpoints.filter((ep) => !ep.conflict && isSkipped(ep)));
168175

169176
return groups;
170-
}, [specAddedEndpoints, specUpdatedEndpoints, localUpdatedEndpoints, specRemovedEndpoints, decisions]);
177+
}, [specAddedEndpoints, specUpdatedEndpoints, specRemovedEndpoints, decisions]);
171178

172179
const handleConfirmApply = () => {
173180
setShowConfirmation(false);
@@ -187,7 +194,6 @@ const SyncReviewPage = ({
187194

188195
onApplySync({
189196
endpointDecisions: decisions,
190-
removedIds: [],
191197
localOnlyIds,
192198
// Pass filtered categorized endpoints for performSync to construct the right backend diff
193199
newToCollection: filteredAddedEndpoints,

packages/bruno-app/src/components/OpenAPISyncTab/hooks/useOpenAPISync.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const useOpenAPISync = (collection) => {
9090

9191
const prevItemCountRef = useRef(httpItemCount);
9292
const isDriftLoadingRef = useRef(false);
93+
const specDriftRef = useRef(specDrift);
9394

9495
const loadCollectionDrift = async ({ clear = false } = {}) => {
9596
if (isDriftLoadingRef.current && !clear) return;
@@ -328,8 +329,31 @@ const useOpenAPISync = (collection) => {
328329
}
329330
};
330331

331-
// Reload drift — passed to useEndpointActions so it can refresh after actions
332-
const reloadDrift = () => loadCollectionDrift({ clear: true });
332+
// Keep ref in sync so reloadDrift always reads the latest specDrift
333+
specDriftRef.current = specDrift;
334+
335+
// Reload both drifts — passed to useEndpointActions so it can refresh after actions.
336+
// Uses specDriftRef to avoid stale closure over specDrift state.
337+
const reloadDrift = async () => {
338+
await loadCollectionDrift({ clear: true });
339+
// Refresh remoteDrift if we have a remote spec cached from the last check
340+
const currentSpecDrift = specDriftRef.current;
341+
if (currentSpecDrift?.newSpec) {
342+
try {
343+
const { ipcRenderer } = window;
344+
const remoteComparison = await ipcRenderer.invoke('renderer:get-collection-drift', {
345+
collectionPath: collection.pathname,
346+
brunoConfig: collection.brunoConfig,
347+
compareSpec: currentSpecDrift.newSpec
348+
});
349+
if (!remoteComparison.error) {
350+
setRemoteDrift(remoteComparison);
351+
}
352+
} catch (err) {
353+
console.error('Error reloading remote drift:', err);
354+
}
355+
}
356+
};
333357

334358
// Save connection settings from the modal
335359
const handleSaveSettings = async ({ sourceUrl: newUrl, autoCheck, autoCheckInterval }) => {

packages/bruno-app/src/components/OpenAPISyncTab/hooks/useSyncFlow.js

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ const useSyncFlow = ({
1414
const [showConfirmModal, setShowConfirmModal] = useState(false);
1515
const [isSyncing, setIsSyncing] = useState(false);
1616

17-
const performSync = async (selections = { removedIds: [], localOnlyIds: [], endpointDecisions: {} }, mode = 'sync') => {
17+
const performSync = async (selections = { localOnlyIds: [], endpointDecisions: {} }, mode = 'sync') => {
1818
setShowConfirmModal(false);
1919
setIsSyncing(true);
2020
setError(null);
2121

2222
const {
23-
removedIds = [], localOnlyIds = [], endpointDecisions: decisions = {},
23+
localOnlyIds = [], endpointDecisions: decisions = {},
2424
newToCollection, specUpdates, resolvedConflicts, localChangesToReset
2525
} = selections;
2626

@@ -49,9 +49,7 @@ const useSyncFlow = ({
4949
// Called from "Sync Now" (skip review) or ConfirmSyncModal — use specDrift directly
5050
filteredDiff = {
5151
...specDrift,
52-
removed: removedIds.length > 0
53-
? (specDrift?.removed || []).filter((ep) => removedIds.includes(ep.id))
54-
: []
52+
removed: [] // Removals handled via localOnlyToRemove
5553
};
5654

5755
localOnlyToRemove = localOnlyIds.length > 0
@@ -69,7 +67,7 @@ const useSyncFlow = ({
6967
collectionPath: collection.pathname,
7068
sourceUrl: sourceUrl.trim(),
7169
addNewRequests: mode !== 'spec-only',
72-
removeDeletedRequests: removedIds.length > 0 || localOnlyIds.length > 0,
70+
removeDeletedRequests: localOnlyIds.length > 0,
7371
diff: filteredDiff,
7472
localOnlyToRemove,
7573
driftedToReset,
@@ -113,10 +111,21 @@ const useSyncFlow = ({
113111
setPendingSyncMode(null);
114112
};
115113

114+
// Only treat endpoints as spec changes if they actually changed in the spec
115+
// (not locally-added/deleted endpoints that were never in or removed from the spec)
116+
const specAddedIds = useMemo(() => {
117+
return new Set((specDrift?.added || []).map((ep) => ep.id));
118+
}, [specDrift]);
119+
120+
const specRemovedIds = useMemo(() => {
121+
return new Set((specDrift?.removed || []).map((ep) => ep.id));
122+
}, [specDrift]);
123+
116124
const handleConfirmModalSync = () => {
117-
const localOnlyIds = (remoteDrift?.localOnly || []).map((ep) => ep.id);
125+
const localOnlyIds = (remoteDrift?.localOnly || [])
126+
.filter((ep) => specRemovedIds.has(ep.id))
127+
.map((ep) => ep.id);
118128
performSync({
119-
removedIds: [],
120129
localOnlyIds,
121130
endpointDecisions: {}
122131
}, pendingSyncMode || 'sync');
@@ -125,17 +134,19 @@ const useSyncFlow = ({
125134
const confirmGroups = useMemo(() => {
126135
if (!remoteDrift) return [];
127136
const groups = [];
128-
if (remoteDrift.missing?.length > 0) {
129-
groups.push({ label: 'New endpoints to add', type: 'add', endpoints: remoteDrift.missing });
137+
const actuallyAdded = (remoteDrift.missing || []).filter((ep) => specAddedIds.has(ep.id));
138+
if (actuallyAdded.length > 0) {
139+
groups.push({ label: 'New endpoints to add', type: 'add', endpoints: actuallyAdded });
130140
}
131141
if (remoteDrift.modified?.length > 0) {
132142
groups.push({ label: 'Endpoints to update', type: 'update', endpoints: remoteDrift.modified });
133143
}
134-
if (remoteDrift.localOnly?.length > 0) {
135-
groups.push({ label: 'Endpoints to delete', type: 'remove', endpoints: remoteDrift.localOnly });
144+
const actuallyRemoved = (remoteDrift.localOnly || []).filter((ep) => specRemovedIds.has(ep.id));
145+
if (actuallyRemoved.length > 0) {
146+
groups.push({ label: 'Endpoints to delete', type: 'remove', endpoints: actuallyRemoved });
136147
}
137148
return groups;
138-
}, [remoteDrift]);
149+
}, [remoteDrift, specAddedIds, specRemovedIds]);
139150

140151
return {
141152
isSyncing, showConfirmModal, confirmGroups,

packages/bruno-app/src/components/OpenAPISyncTab/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ const OpenAPISyncTab = ({ collection }) => {
130130
onTabSelect={setActiveTab}
131131
error={error}
132132
isLoading={isLoading}
133-
fileNotFound={fileNotFound}
134133
onOpenSettings={() => setShowSettingsModal(true)}
135134
/>
136135
<p className="beta-feedback-inline">

0 commit comments

Comments
 (0)