fix: insomnia open dialog for proto directory can't select directories (#3348)

* fix: insomnia open dialog for proto directory can't select directories

* uses a named export for selectFileOrFolder

(also, removes original js file from rebase)

* clears error by leveraging exhaustiveness check

* fixes bug: the `name` field is actually for a file filter

see the referenced pull request.

As for the `extensions: ['*']`, there's no reason I can see to include a filter and then tell the filter to then accept everything.

* update selectFileOrFolder mocks

* use switch (for exhaustiveness checking) and type selectedFormat

* removes unnecessary filters from _save_ dialog

from the docs:
> The filters specifies an array of file types that can be displayed

As suspected, this is not needed.  A user is free to save it wherever they want.

* adds extension to saved file

not sure why this was missing before, but it appears to have been a bug

* formatting updates

best to "ignore whitespace" for this commit.  I did this with the hope of using the `ThunkAction` type from `redux-thunk`, but once I got them all looking good and started adding the type I quickly learned there's quite a bit more work to do in this area before we can have such a thing.  I therefore opted to just call it a day at that and take the (no-op) formatting changes and typings.

* removes remaining name filters from save dialogs

same reason as the 2nd to prior commit - they cause the bug

Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com>
Co-authored-by: Opender Singh <opender.singh@konghq.com>
This commit is contained in:
Akhil Sasidharan 2021-05-26 19:54:21 +05:30 committed by GitHub
parent 3c881d5117
commit 0b38c68c1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 491 additions and 519 deletions

View File

@ -1,14 +1,16 @@
import { OpenDialogOptions, remote } from 'electron';
interface Options {
itemTypes?: ('file' | 'directory')[];
extensions?: string[];
}
interface FileSelection {
filePath: string;
canceled: boolean;
}
const selectFileOrFolder = async ({ itemTypes, extensions }: Options): Promise<FileSelection> => {
export const selectFileOrFolder = async ({ itemTypes, extensions }: Options) => {
// If no types are selected then default to just files and not directories
const types = itemTypes || ['file'];
let title = 'Select ';
@ -26,41 +28,27 @@ const selectFileOrFolder = async ({ itemTypes, extensions }: Options): Promise<F
}
const options: OpenDialogOptions = {
title: title,
title,
buttonLabel: 'Select',
// @ts-expect-error -- TSCONVERSION we should update this to accept other properties types as well, which flow all the way up to plugins
properties: types.map(type => {
if (type === 'file') {
return 'openFile';
}
switch (type) {
case 'file':
return 'openFile';
if (type === 'directory') {
return 'openDirectory';
case 'directory':
return 'openDirectory';
}
}),
filters: [
{
name: 'All Files',
extensions: ['*'],
},
],
// @ts-expect-error https://github.com/electron/electron/pull/29322
filters: [{
extensions: (extensions?.length ? extensions : ['*']),
}],
};
// If extensions are provided then filter for just those extensions
if (extensions?.length) {
options.filters = [
{
name: 'Files',
extensions: extensions,
},
];
}
const { canceled, filePaths } = await remote.dialog.showOpenDialog(options);
return {
const fileSelection: FileSelection = {
filePath: filePaths[0],
canceled,
};
return fileSelection;
};
export default selectFileOrFolder;

View File

@ -5,23 +5,27 @@ import * as protoManager from '../proto-manager';
import * as protoLoader from '../proto-loader';
import * as models from '../../../models';
import { globalBeforeEach } from '../../../__jest__/before-each';
import selectFileOrFolder from '../../../common/select-file-or-folder';
import { selectFileOrFolder as _selectFileOrFolder } from '../../../common/select-file-or-folder';
jest.mock('../../../common/select-file-or-folder', () => ({
__esModule: true,
default: jest.fn(),
selectFileOrFolder: jest.fn(),
}));
const selectFileOrFolder = _selectFileOrFolder as jest.MockedFunction<typeof _selectFileOrFolder>;
describe('proto management integration test', () => {
beforeEach(globalBeforeEach);
it('can ingest proto file and load methods from it', async () => {
const w = await models.workspace.create();
// Mock folder selection
const protoFilePath = path.join(__dirname, '../__fixtures__/library/hello.proto');
selectFileOrFolder.mockResolvedValue({
filePath: protoFilePath,
canceled: false,
});
// Ingest into database
let createdProtoFileId;
await protoManager.addFile(w._id, id => {
@ -31,8 +35,10 @@ describe('proto management integration test', () => {
itemTypes: ['file'],
extensions: ['proto'],
});
// Find proto file entries
const helloProto = await models.protoFile.getById(createdProtoFileId);
// Load protoMethods
const helloMethods = await protoLoader.loadMethods(helloProto);
expect(helloMethods.length).toBe(4);
@ -40,22 +46,27 @@ describe('proto management integration test', () => {
it('can ingest proto directory tree and load methods from any file', async () => {
const w = await models.workspace.create();
// Mock folder selection
const libraryDirPath = path.join(__dirname, '../__fixtures__/library');
selectFileOrFolder.mockResolvedValue({
filePath: libraryDirPath,
canceled: false,
});
// Ingest into database
await protoManager.addDirectory(w._id);
expect(selectFileOrFolder).toHaveBeenCalledWith({
itemTypes: ['directory'],
extensions: ['proto'],
});
// Find proto file entries
const protoFiles = await models.protoFile.all();
const rootProto = protoFiles.find(pf => pf.name === 'root.proto');
const helloProto = protoFiles.find(pf => pf.name === 'hello.proto');
const timeProto = protoFiles.find(pf => pf.name === 'time.proto');
// Load protoMethods
const rootMethods = await protoLoader.loadMethods(rootProto);
const helloMethods = await protoLoader.loadMethods(helloProto);
@ -63,15 +74,17 @@ describe('proto management integration test', () => {
expect(rootMethods.length).toBe(helloMethods.length + timeMethods.length);
expect(helloMethods.length).toBe(4);
expect(timeMethods.length).toBe(1);
// Create request
const gr = await models.grpcRequest.create({
parentId: w._id,
protoFileId: rootProto._id,
protoFileId: rootProto?._id,
protoMethodName: rootMethods[0].path,
});
// Load selected method
const selectedMethod = await protoLoader.getSelectedMethod(gr);
expect(selectedMethod.originalName).toEqual(rootMethods[0].originalName);
expect(selectedMethod?.originalName).toEqual(rootMethods[0].originalName);
});
afterEach(async () => {

View File

@ -1,7 +1,7 @@
import fs from 'fs';
import path from 'path';
import { globalBeforeEach } from '../../../../__jest__/before-each';
import selectFileOrFolder from '../../../../common/select-file-or-folder';
import { selectFileOrFolder } from '../../../../common/select-file-or-folder';
import * as protoManager from '../index';
import { loadMethods as _loadMethods, loadMethodsFromPath as _loadMethodsFromPath } from '../../proto-loader';
import * as models from '../../../../models';
@ -12,9 +12,9 @@ const loadMethods = _loadMethods as jest.Mock;
const loadMethodsFromPath = _loadMethodsFromPath as jest.Mock;
jest.mock('../../../../common/select-file-or-folder', () => ({
__esModule: true,
default: jest.fn(),
selectFileOrFolder: jest.fn(),
}));
jest.mock('../../../../ui/components/modals');
jest.mock('../../proto-loader');
@ -109,9 +109,9 @@ describe('protoManager', () => {
// Assert
const pf = await models.protoFile.getByParentId(w._id);
expect(cbMock).toHaveBeenCalledWith(pf._id);
expect(pf.name).toBe(filePath);
expect(pf.protoText).toBe(contents);
expect(cbMock).toHaveBeenCalledWith(pf?._id);
expect(pf?.name).toBe(filePath);
expect(pf?.protoText).toBe(contents);
});
});
@ -138,8 +138,8 @@ describe('protoManager', () => {
// Assert
expect(cbMock).toHaveBeenCalledWith(pf._id);
const updatedPf = await models.protoFile.getById(pf._id);
expect(updatedPf.name).toBe(filePath);
expect(updatedPf.protoText).toBe(contents);
expect(updatedPf?.name).toBe(filePath);
expect(updatedPf?.protoText).toBe(contents);
});
});
@ -158,7 +158,7 @@ describe('protoManager', () => {
// Assert
const updatedPf = await models.protoFile.getById(pf._id);
expect(updatedPf.name).toBe(updatedName);
expect(updatedPf?.name).toBe(updatedName);
});
});

View File

@ -4,7 +4,7 @@ import * as models from '../../../models';
import React from 'react';
import type { ProtoDirectory } from '../../../models/proto-directory';
import { database as db } from '../../../common/database';
import selectFileOrFolder from '../../../common/select-file-or-folder';
import { selectFileOrFolder } from '../../../common/select-file-or-folder';
import ingestProtoDirectory from './ingest-proto-directory';
import fs from 'fs';
import path from 'path';

View File

@ -2,7 +2,7 @@ import React, { HTMLAttributes, PureComponent } from 'react';
import { autoBindMethodsForReact } from 'class-autobind-decorator';
import { AUTOBIND_CFG } from '../../../common/constants';
import { basename as pathBasename } from 'path';
import selectFileOrFolder from '../../../common/select-file-or-folder';
import { selectFileOrFolder } from '../../../common/select-file-or-folder';
interface Props extends Omit<HTMLAttributes<HTMLButtonElement>, 'onChange'> {
onChange: (path: string) => void;

View File

@ -1,5 +1,5 @@
import React, { PureComponent } from 'react';
import * as electron from 'electron';
import electron, { SaveDialogOptions } from 'electron';
import mimes from 'mime-types';
import fs from 'fs';
import moment from 'moment';
@ -135,13 +135,13 @@ class ResponseMultipart extends PureComponent<Props, State> {
const dir = lastDir || electron.remote.app.getPath('desktop');
const date = moment().format('YYYY-MM-DD');
const filename = part.filename || `${part.name}_${date}`;
const options = {
const options: SaveDialogOptions = {
title: 'Save as File',
buttonLabel: 'Save',
defaultPath: path.join(dir, filename),
filters: [
// @ts-expect-error https://github.com/electron/electron/pull/29322
{
name: 'Download',
extensions: [extension],
},
],

File diff suppressed because it is too large Load Diff