Skip to content

feat(compass-saved-aggregations-queries): update namespace for saved aggregations/queries (COMPASS-7665) #5462

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
merged 6 commits into from
Feb 27, 2024
Merged

feat(compass-saved-aggregations-queries): update namespace for saved aggregations/queries (COMPASS-7665) #5462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cd709ba
let users update queries/aggregations
baileympearson Feb 16, 2024
20d4553
add e2e tests for selecting a new namespace
baileympearson Feb 20, 2024
0ce0217
Fix lint
baileympearson Feb 20, 2024
422808f
account for optional pipeline storage and query storage
baileympearson Feb 20, 2024
43c9ccd
store update namesapce information in store
baileympearson Feb 23, 2024
25ad914
use connect + mapToDispatch instead of useDispatch
baileympearson Feb 26, 2024
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
113 changes: 113 additions & 0 deletions packages/compass-e2e-tests/tests/instance-my-queries-tab.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,117 @@ describe('Instance my queries tab', function () {
const namespace = await browser.getActiveTabNamespace();
expect(namespace).to.equal('test.numbers');
});

context(
'when a user has a saved query associated with a collection that does not exist',
function () {
const favoriteQueryName = 'list of numbers greater than 10 - query';
const newCollectionName = 'numbers-renamed';

/** saves a query and renames the collection associated with the query, so that the query must be opened with the "select namespace" modal */
async function setup() {
// Run a query
await browser.navigateToCollectionTab('test', 'numbers', 'Documents');
await browser.runFindOperation('Documents', `{i: {$gt: 10}}`, {
limit: '10',
});
await browser.clickVisible(Selectors.QueryBarHistoryButton);

// Wait for the popover to show
const history = await browser.$(Selectors.QueryBarHistory);
await history.waitForDisplayed();

// wait for the recent item to show.
const recentCard = await browser.$(Selectors.QueryHistoryRecentItem);
await recentCard.waitForDisplayed();

// Save the ran query
await browser.hover(Selectors.QueryHistoryRecentItem);
await browser.clickVisible(Selectors.QueryHistoryFavoriteAnItemButton);
await browser.setValueVisible(
Selectors.QueryHistoryFavoriteItemNameField,
favoriteQueryName
);
await browser.clickVisible(
Selectors.QueryHistorySaveFavoriteItemButton
);

await browser.closeWorkspaceTabs();
await browser.navigateToInstanceTab('Databases');
await browser.navigateToInstanceTab('My Queries');

// open the menu
await openMenuForQueryItem(browser, favoriteQueryName);

// copy to clipboard
await browser.clickVisible(Selectors.SavedItemMenuItemCopy);

if (process.env.COMPASS_E2E_DISABLE_CLIPBOARD_USAGE !== 'true') {
await browser.waitUntil(
async () => {
const text = (await clipboard.read())
.replace(/\s+/g, ' ')
.replace(/\n/g, '');
const isValid =
text ===
'{ "collation": null, "filter": { "i": { "$gt": 10 } }, "limit": 10, "project": null, "skip": null, "sort": null }';
if (!isValid) {
console.log(text);
}
return isValid;
},
{ timeoutMsg: 'Expected copy to clipboard to work' }
);
}

// rename the collection associated with the query to force the open item modal
await browser.shellEval('use test');
await browser.shellEval(
`db.numbers.renameCollection('${newCollectionName}')`
);
await browser.clickVisible(Selectors.SidebarRefreshDatabasesButton);
}
beforeEach(setup);

it('users can permanently associate a new namespace for an aggregation/query', async function () {
await browser.navigateToInstanceTab('My Queries');
// browse to the query
await browser.clickVisible(Selectors.myQueriesItem(favoriteQueryName));

// the open item modal - select a new collection
const openModal = await browser.$(Selectors.OpenSavedItemModal);
await openModal.waitForDisplayed();
await browser.selectOption(
Selectors.OpenSavedItemDatabaseField,
'test'
);
await browser.selectOption(
Selectors.OpenSavedItemCollectionField,
newCollectionName
);

await browser.clickParent(
'[data-testid="update-query-aggregation-checkbox"]'
);

const confirmOpenButton = await browser.$(
Selectors.OpenSavedItemModalConfirmButton
);
await confirmOpenButton.waitForEnabled();

await confirmOpenButton.click();
await openModal.waitForDisplayed({ reverse: true });

await browser.navigateToInstanceTab('My Queries');

const [databaseNameElement, collectionNameElement] = [
await browser.$('span=test'),
await browser.$(`span=${newCollectionName}`),
];

await databaseNameElement.waitForDisplayed();
await collectionNameElement.waitForDisplayed();
});
}
);
});
34 changes: 31 additions & 3 deletions packages/compass-saved-aggregations-queries/src/components/open-item-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import {
Checkbox,
FormModal,
Option,
Select,
Expand All @@ -14,6 +15,7 @@ import {
openSelectedItem,
selectCollection,
selectDatabase,
updateItemNamespaceChecked,
} from '../stores/open-item';

type AsyncItemsSelectProps = {
Expand Down Expand Up @@ -98,15 +100,18 @@ type OpenItemModalProps = {
itemName: string;
isModalOpen: boolean;
isSubmitDisabled: boolean;
updateItemNamespace: boolean;
onSubmit(): void;
onClose(): void;
onUpdateNamespaceChecked(checked: boolean): void;
};

const modalContent = css({
display: 'grid',
gridTemplateAreas: `
'description description'
'database collection'
'checkbox checkbox'
`,
gridAutoColumns: '1fr',
rowGap: spacing[4],
Expand All @@ -125,20 +130,26 @@ const collectionSelect = css({
gridArea: 'collection',
});

const checkbox = css({
gridArea: 'checkbox',
});

const OpenItemModal: React.FunctionComponent<OpenItemModalProps> = ({
namespace,
itemType,
itemName,
isModalOpen,
isSubmitDisabled,
updateItemNamespace,
onClose,
onSubmit,
onUpdateNamespaceChecked,
}) => {
return (
<FormModal
open={isModalOpen}
onCancel={onClose}
onSubmit={onSubmit}
onSubmit={() => onSubmit()}
title="Select a Namespace"
submitButtonText="Open"
submitDisabled={isSubmitDisabled}
Expand All @@ -161,6 +172,15 @@ const OpenItemModal: React.FunctionComponent<OpenItemModalProps> = ({
label="Collection"
></CollectionSelect>
</div>
<Checkbox
className={checkbox}
checked={updateItemNamespace}
onChange={(event) => {
onUpdateNamespaceChecked(event.target.checked);
}}
label={`Update this ${itemType} with the newly selected namespace`}
data-testid="update-query-aggregation-checkbox"
/>
</div>
</FormModal>
);
Expand All @@ -169,7 +189,12 @@ const OpenItemModal: React.FunctionComponent<OpenItemModalProps> = ({
const mapState: MapStateToProps<
Pick<
OpenItemModalProps,
'isModalOpen' | 'isSubmitDisabled' | 'namespace' | 'itemType' | 'itemName'
| 'isModalOpen'
| 'isSubmitDisabled'
| 'namespace'
| 'itemType'
| 'itemName'
| 'updateItemNamespace'
>,
Record<string, never>,
RootState
Expand All @@ -179,6 +204,7 @@ const mapState: MapStateToProps<
selectedDatabase,
selectedCollection,
selectedItem: item,
updateItemNamespace,
},
}) => {
return {
Expand All @@ -187,15 +213,17 @@ const mapState: MapStateToProps<
namespace: `${item?.database ?? ''}.${item?.collection ?? ''}`,
itemName: item?.name ?? '',
itemType: item?.type ?? '',
updateItemNamespace,
};
};

const mapDispatch: MapDispatchToProps<
Pick<OpenItemModalProps, 'onSubmit' | 'onClose'>,
Pick<OpenItemModalProps, 'onSubmit' | 'onClose' | 'onUpdateNamespaceChecked'>,
Record<string, never>
> = {
onSubmit: openSelectedItem,
onClose: closeModal,
onUpdateNamespaceChecked: updateItemNamespaceChecked,
};

export default connect(mapState, mapDispatch)(OpenItemModal);
53 changes: 50 additions & 3 deletions packages/compass-saved-aggregations-queries/src/stores/open-item.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type State = {
collections: string[];
selectedCollection: string | null;
collectionsStatus: Status;
updateItemNamespace: boolean;
};

const INITIAL_STATE: State = {
Expand All @@ -26,6 +27,7 @@ const INITIAL_STATE: State = {
collections: [],
selectedCollection: null,
collectionsStatus: 'initial',
updateItemNamespace: false,
};

export enum ActionTypes {
Expand All @@ -40,6 +42,7 @@ export enum ActionTypes {
LoadCollections = 'compass-saved-aggregations-queries/loadCollections',
LoadCollectionsSuccess = 'compass-saved-aggregations-queries/loadCollectionsSuccess',
LoadCollectionsError = 'compass-saved-aggregations-queries/loadCollectionsError',
UpdateNamespaceChecked = 'compass-saved-aggregations-queries/updateNamespaceChecked',
}

type OpenModalAction = {
Expand Down Expand Up @@ -92,6 +95,11 @@ type LoadCollectionsErrorAction = {
type: ActionTypes.LoadCollectionsError;
};

type UpdateNamespaceChecked = {
type: ActionTypes.UpdateNamespaceChecked;
updateItemNamespace: boolean;
};

export type Actions =
| OpenModalAction
| CloseModalAction
Expand All @@ -103,7 +111,8 @@ export type Actions =
| SelectCollectionAction
| LoadCollectionsAction
| LoadCollectionsErrorAction
| LoadCollectionsSuccessAction;
| LoadCollectionsSuccessAction
| UpdateNamespaceChecked;

const reducer: Reducer<State> = (state = INITIAL_STATE, action) => {
switch (action.type) {
Expand Down Expand Up @@ -165,11 +174,21 @@ const reducer: Reducer<State> = (state = INITIAL_STATE, action) => {
collections: action.collections,
collectionsStatus: 'ready',
};
case ActionTypes.UpdateNamespaceChecked:
return {
...state,
updateItemNamespace: action.updateItemNamespace,
};
default:
return state;
}
};

export const updateItemNamespaceChecked = (updateItemNamespace: boolean) => ({
type: ActionTypes.UpdateNamespaceChecked,
updateItemNamespace,
});

const openModal =
(selectedItem: Item): SavedQueryAggregationThunkAction<Promise<void>> =>
async (dispatch, _getState, { instance, dataService }) => {
Expand Down Expand Up @@ -249,16 +268,44 @@ export const openSavedItem =
dispatch(openItem(item, database, collection));
};

export const updateNamespaceChecked =
(updateNamespaceChecked: boolean): SavedQueryAggregationThunkAction<void> =>
(dispatch) => {
dispatch({
type: ActionTypes.UpdateNamespaceChecked,
updateNamespaceChecked,
});
};

export const openSelectedItem =
(): SavedQueryAggregationThunkAction<void> => (dispatch, getState) => {
(): SavedQueryAggregationThunkAction<Promise<void>> =>
async (dispatch, getState, { queryStorage, pipelineStorage }) => {
const {
openItem: { selectedItem, selectedDatabase, selectedCollection },
openItem: {
selectedItem,
selectedDatabase,
selectedCollection,
updateItemNamespace,
},
} = getState();

if (!selectedItem || !selectedDatabase || !selectedCollection) {
return;
}

if (updateItemNamespace) {
const id = selectedItem.id;
const newNamespace = `${selectedDatabase}.${selectedCollection}`;

if (selectedItem.type === 'aggregation') {
await pipelineStorage?.updateAttributes(id, {
namespace: newNamespace,
});
} else if (selectedItem.type === 'query') {
await queryStorage?.updateAttributes(id, { _ns: newNamespace });
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we shouldn't dispatch some 'itemUpdated' action here. probably not necessary, it just feels weird that there would be no trace of this change happening in redux.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, I'd say that as in this case there is no effect on the actual reducer and we already dispatch an action that you can trace back to this handler, it's okay not to do this. Maybe if there was even more logic involved and the state of the update was part of the reducer, this would make more sense to do. Good point though!

}

dispatch({ type: ActionTypes.CloseModal });
dispatch(openItem(selectedItem, selectedDatabase, selectedCollection));
};
Expand Down