gabotorresruiz commented on code in PR #43216:
URL: https://github.com/apache/superset/pull/43216#discussion_r3806865018
##########
superset-frontend/src/SqlLab/reducers/sqlLab.test.ts:
##########
@@ -61,6 +61,67 @@ describe('sqlLabReducer', () => {
});
});
+ test('should default extra_json to an empty object when extra is unset', ()
=> {
+ const incomingDb = {
+ ...databases.result[0],
+ extra: null,
+ };
+ const incomingDbId = Number(incomingDb.id);
+
+ const action = actions.setDatabases([incomingDb] as any);
+
+ const newState = sqlLabReducer(initialState, action);
+
+ expect(newState.databases[incomingDbId]).toEqual({
+ ...incomingDb,
+ extra_json: {},
+ });
+ });
+
+ test('defaults extra_json when a database has no extra (#43216)', () => {
Review Comment:
Just a small NIT: this test and the one at line 64 are the same case, both
build `databases.result[0]` with `extra: null` and assert `extra_json` defaults
to `{}` (Bito flagged this too). Could you keep just one? My preference would
be keeping the first test's descriptive name, moving this explanatory comment
onto it, and dropping the PR number from the name since git blame already links
back here. That also makes the suite match the three new cases the PR
description lists.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
