Skip to content

Commit

Permalink
chore: Various typing fixes (#754)
Browse files Browse the repository at this point in the history
Signed-off-by: Marcos Iglesias Valle <[email protected]>
  • Loading branch information
dorianj authored Nov 4, 2020
1 parent 37ee764 commit 4794dfb
Show file tree
Hide file tree
Showing 18 changed files with 146 additions and 188 deletions.
105 changes: 9 additions & 96 deletions amundsen_application/static/.betterer.results

Large diffs are not rendered by default.

12 changes: 7 additions & 5 deletions amundsen_application/static/js/components/NavBar/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,20 @@ describe('NavBar', () => {

describe('renderSearchBar', () => {
it('returns small SearchBar when not on home page', () => {
const { props, wrapper } = setup(null, { pathname: '/search' });
const searchBar = shallow(wrapper.instance().renderSearchBar()).find(
SearchBar
);
const { props, wrapper } = setup(undefined, { pathname: '/search' });
const rendered = wrapper.instance().renderSearchBar();
if (rendered === null) {
throw Error('rendering search bar returned null');
}
const searchBar = shallow(rendered).find(SearchBar);
expect(searchBar.exists()).toBe(true);
expect(searchBar.props()).toMatchObject({
size: 'small',
});
});

it('returns null if conditions to render search bar are not met', () => {
const { props, wrapper } = setup(null, { pathname: '/' });
const { props, wrapper } = setup(undefined, { pathname: '/' });
expect(wrapper.instance().renderSearchBar()).toBe(null);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ describe('OwnerEditor', () => {
errorText: null,
isLoading: false,
itemProps: {},
isEditing: null,
isEditing: undefined,
setEditMode: jest.fn(),
onUpdateList: jest.fn(),
readOnly: null,
readOnly: undefined,
resourceType: ResourceType.table,
...propOverrides,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,18 @@ export class OwnerEditor extends React.Component<

recordAddItem = (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
const { value } = this.inputRef.current;
if (this.inputRef.current) {
const { value } = this.inputRef.current;

if (this.inputRef.current && value) {
this.inputRef.current.value = '';
if (value) {
this.inputRef.current.value = '';

const newTempItemProps = {
...this.state.tempItemProps,
[value]: { label: value },
};
this.setState({ tempItemProps: newTempItemProps });
const newTempItemProps = {
...this.state.tempItemProps,
[value]: { label: value },
};
this.setState({ tempItemProps: newTempItemProps });
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ describe('SearchItem', () => {

it('renders correct text if !props.hasResults', () => {
const { wrapper } = setup({ hasResults: false });
const content = shallow(wrapper.instance().renderIndicator());
expect(content.text()).toBe(SEARCH_ITEM_NO_RESULTS);
const rendered = wrapper.instance().renderIndicator();
if (rendered === null) {
throw Error('renderIndicator returned null');
}
expect(shallow(rendered).text()).toBe(SEARCH_ITEM_NO_RESULTS);
});

it('renders nothing if !props.Loading and props.hasResults', () => {
Expand Down
31 changes: 22 additions & 9 deletions amundsen_application/static/js/ducks/bookmark/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,29 +121,41 @@ export default function reducer(
): BookmarkReducerState {
switch (action.type) {
case AddBookmark.SUCCESS:
case GetBookmarks.SUCCESS:
case GetBookmarks.SUCCESS: {
const { payload } = <GetBookmarksResponse>action;
if (payload === undefined) {
throw Error('payload must be set for GetBookmarks.SUCCESS');
}
return {
...state,
myBookmarks: (<GetBookmarksResponse>action).payload.bookmarks,
myBookmarks: payload.bookmarks,
myBookmarksIsLoaded: true,
};
}
case GetBookmarksForUser.REQUEST:
return {
...state,
bookmarksForUser: {
...initialBookmarkState,
},
};
case GetBookmarksForUser.SUCCESS:
case GetBookmarksForUser.SUCCESS: {
const { payload } = <GetBookmarksForUserResponse>action;

if (payload === undefined) {
throw Error('payload must be set for GetBookmarksForUser.SUCCESS');
}
return {
...state,
bookmarksForUser: (<GetBookmarksForUserResponse>action).payload
.bookmarks,
bookmarksForUser: payload.bookmarks,
};
case RemoveBookmark.SUCCESS:
const { resourceKey, resourceType } = (<RemoveBookmarkResponse>(
action
)).payload;
}
case RemoveBookmark.SUCCESS: {
const { payload } = <RemoveBookmarkResponse>action;
if (payload === undefined) {
throw Error('payload must be set for RemoveBookmark.SUCCESS');
}
const { resourceKey, resourceType } = payload;
return {
...state,
myBookmarks: {
Expand All @@ -153,6 +165,7 @@ export default function reducer(
),
},
};
}
case AddBookmark.FAILURE:
case GetBookmarks.FAILURE:
case GetBookmarksForUser.FAILURE:
Expand Down
14 changes: 12 additions & 2 deletions amundsen_application/static/js/ducks/search/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ export interface SearchReducerState {
export function searchAll(
searchType: SearchType,
term: string,
resource?: ResourceType,
pageIndex?: number,
resource: ResourceType,
pageIndex: number,
useFilters: boolean = false
): SearchAllRequest {
return {
Expand Down Expand Up @@ -314,6 +314,11 @@ export default function reducer(
case SearchAll.SUCCESS:
// resets all resources with initial state then applies search results
const newState = (<SearchAllResponse>action).payload;
if (newState === undefined) {
throw Error(
'SearchAllResponse.payload must be specified for SUCCESS type'
);
}
return {
...initialState,
...newState,
Expand Down Expand Up @@ -354,6 +359,11 @@ export default function reducer(
};
case InlineSearch.SUCCESS:
const inlineResults = (<InlineSearchResponse>action).payload;
if (inlineResults === undefined) {
throw Error(
'InlineSearchResponse.payload must be specified for SUCCESS type'
);
}
return {
...state,
inlineResults: {
Expand Down
24 changes: 13 additions & 11 deletions amundsen_application/static/js/ducks/search/sagas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,19 @@ export function* submitSearchResourceWatcher(): SagaIterator {
export function* updateSearchStateWorker(
action: UpdateSearchStateRequest
): SagaIterator {
const { filters, resource, updateUrl, submitSearch } = action.payload;
const state = yield select(getSearchState);
if (filters && submitSearch) {
yield put(searchAll(SearchType.FILTER, '', undefined, 0, true));
} else if (updateUrl) {
updateSearchUrl({
resource: resource || state.resource,
term: state.search_term,
index: getPageIndex(state, resource),
filters: filters || state.filters,
});
if (action.payload !== undefined) {
const { filters, resource, updateUrl, submitSearch } = action.payload;
const state = yield select(getSearchState);
if (filters && submitSearch) {
yield put(searchAll(SearchType.FILTER, '', undefined, 0, true));
} else if (updateUrl) {
updateSearchUrl({
resource: resource || state.resource,
term: state.search_term,
index: getPageIndex(state, resource),
filters: filters || state.filters,
});
}
}
}
export function* updateSearchStateWatcher(): SagaIterator {
Expand Down
6 changes: 3 additions & 3 deletions amundsen_application/static/js/ducks/search/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ export const getPageIndex = (
resource = resource || state.resource;
switch (resource) {
case ResourceType.table:
return state.tables.page_index;
return state.tables?.page_index || 0;
case ResourceType.user:
return state.users.page_index;
return state.users?.page_index || 0;
case ResourceType.dashboard:
return state.dashboards.page_index;
return state.dashboards?.page_index || 0;
}
return 0;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('helpers', () => {
it('returns false if not a user with display_name', () => {
const testUser = {
...globalState.user.loggedInUser,
display_name: null,
display_name: '',
};
expect(Helpers.shouldSendNotification(testUser)).toBe(false);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ describe('tableMetadata ducks', () => {
sagaTest = (mockSuccess) => {
return testSaga(
updateTableDescriptionWorker,
updateTableDescription(newDescription, mockSuccess, null)
updateTableDescription(newDescription, mockSuccess, undefined)
)
.next()
.select()
Expand All @@ -510,7 +510,7 @@ describe('tableMetadata ducks', () => {
sagaTest = (mockFailure) => {
return testSaga(
updateTableDescriptionWorker,
updateTableDescription(newDescription, null, mockFailure)
updateTableDescription(newDescription, undefined, mockFailure)
)
.next()
.select()
Expand Down Expand Up @@ -623,7 +623,7 @@ describe('tableMetadata ducks', () => {
newDescription,
columnIndex,
mockSuccess,
null
undefined
)
)
.next()
Expand Down Expand Up @@ -655,7 +655,7 @@ describe('tableMetadata ducks', () => {
updateColumnDescription(
newDescription,
columnIndex,
null,
undefined,
mockFailure
)
)
Expand Down
2 changes: 1 addition & 1 deletion amundsen_application/static/js/fixtures/globalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const globalState: GlobalState = {
},
issue: {
issues: [],
allIssuesUrl: null,
allIssuesUrl: '',
total: 0,
isLoading: true,
},
Expand Down
54 changes: 26 additions & 28 deletions amundsen_application/static/js/fixtures/metadata/users.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,36 @@
export const activeUser0 = {
manager_id: null,
manager_fullname: null,
manager_email: null,
import { PeopleUser } from 'interfaces/User';

export const activeUser0: PeopleUser = {
manager_fullname: undefined,
manager_email: undefined,
profile_url: '/test0',
role_name: null,
display_name: null,
github_username: null,
team_name: null,
last_name: null,
full_name: null,
slack_id: null,
first_name: null,
employee_type: null,
other_key_values: {},
role_name: undefined,
display_name: '',
github_username: '',
team_name: '',
last_name: '',
full_name: '',
slack_id: '',
first_name: '',
employee_type: '',
is_active: true,
email: '[email protected]',
user_id: 'user0',
};

export const activeUser1 = {
manager_id: null,
manager_fullname: null,
manager_email: null,
export const activeUser1: PeopleUser = {
manager_fullname: undefined,
manager_email: undefined,
profile_url: '/test1',
role_name: null,
display_name: null,
github_username: null,
team_name: null,
last_name: null,
full_name: null,
slack_id: null,
first_name: null,
employee_type: null,
other_key_values: {},
role_name: undefined,
display_name: '',
github_username: '',
team_name: '',
last_name: '',
full_name: '',
slack_id: '',
first_name: '',
employee_type: '',
is_active: true,
email: '[email protected]',
user_id: 'user1',
Expand Down
7 changes: 5 additions & 2 deletions amundsen_application/static/js/fixtures/mockRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as History from 'history';
// Mock React-Router
export function getMockRouterProps<P>(
data: P,
location: Partial<History.Location>
location: Partial<History.Location> = {}
): RouteComponentProps<P> {
const mockLocation: History.Location = {
hash: '',
Expand All @@ -24,7 +24,10 @@ export function getMockRouterProps<P>(
url: '',
},
location: mockLocation,
history: {
// This history object is a mock and `null`s many of the required methods. The
// tests are designed not to trigger them, if they do, an error is expected.
// So cast this as any.
history: <any>{
length: 2,
action: 'POP',
location: mockLocation,
Expand Down
12 changes: 6 additions & 6 deletions amundsen_application/static/js/fixtures/search/inlineResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export const allResourcesExample = {
{
display_name: 'Test User',
email: '[email protected]',
employee_type: null,
employee_type: '',
first_name: 'Test',
full_name: 'Test User',
github_username: '',
Expand All @@ -108,16 +108,16 @@ export const allResourcesExample = {
manager_email: '[email protected]',
manager_fullname: 'Test User2',
profile_url: '',
role_name: null,
slack_id: null,
role_name: undefined,
slack_id: '',
team_name: 'Amundsen Team',
type: 'user',
user_id: '[email protected]',
},
{
display_name: 'Test User2',
email: '[email protected]',
employee_type: null,
employee_type: '',
first_name: 'Test',
full_name: 'Test User2',
github_username: '',
Expand All @@ -126,8 +126,8 @@ export const allResourcesExample = {
manager_email: '[email protected]',
manager_fullname: 'Test User3',
profile_url: '',
role_name: null,
slack_id: null,
role_name: undefined,
slack_id: '',
team_name: 'Amundsen Team',
type: 'user',
user_id: '[email protected]',
Expand Down
Loading

0 comments on commit 4794dfb

Please sign in to comment.