rusackas commented on code in PR #40685:
URL: https://github.com/apache/superset/pull/40685#discussion_r3343406933


##########
superset-frontend/src/dashboard/components/PropertiesModal/utils.test.ts:
##########
@@ -60,13 +60,14 @@ test('builds a new owner from the option text label when 
not already in state',
 });
 
 test('falls back to a string label when the option has no text label', () => {
+  // No option in the cache => email stays undefined (not an empty string).
   expect(
     parseSelectedOwners([{ value: 4, label: 'Plain Name' }], [], []),
-  ).toEqual([{ id: 4, full_name: 'Plain Name', email: '' }]);
+  ).toEqual([{ id: 4, full_name: 'Plain Name', email: undefined }]);
 });
 
 test('yields an empty name for a non-string label with no text label', () => {
   expect(parseSelectedOwners([{ value: 5, label: 1 }], [], [])).toEqual([
-    { id: 5, full_name: '', email: '' },
+    { id: 5, full_name: '', email: undefined },
   ]);
 });

Review Comment:
   Good catch — added exactly this case in 780b68f58d (`leaves email undefined 
when option is cached but carries no email`), so the `?? undefined` branch is 
now exercised directly. Thanks!



-- 
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]

Reply via email to