[$1000] [Onyx re-key] Prop type warnings / errors #20802

Beamanator · 2023-06-15T11:34:28Z

Here I'll list a few places people have reported proptypes being incorrect:

This only occurs in main for now, but we should get it fixed soon

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0144288ee67ea3d1ae
Upwork Job ID: 1676295377429790720
Last Price Increase: 2023-07-04

The text was updated successfully, but these errors were encountered:

melvin-bot · 2023-06-15T11:34:33Z

Triggered auto assignment to @NicMendonca (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-06-15T11:34:35Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

alitoshmatov · 2023-06-15T11:55:20Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

[Onyx re-key] Prop type warnings / errors

What is the root cause of that problem?

Before accountID from personalDetails was number, now it is string in personalDetailsList in several places, for example:

App/src/components/DisplayNames/displayNamesPropTypes.js

Line 14 in d396430

accountID: PropTypes.number,

What changes do you think we should make in order to solve the problem?

We should update propTypes of every effected components, changing accountID from number to string

What alternative solutions did you explore? (Optional)

NicMendonca · 2023-06-16T19:05:48Z

@Beamanator what should I be doing with this one? 👀

Beamanator · 2023-06-19T08:56:47Z

haha good question 😬 I think we can keep this open for a bit, but hopefully soon all of these will be resolved - I'll assign myself too to keep an eye out

NicMendonca · 2023-06-23T19:29:22Z

@Beamanator heads up I am going OOO until Wednesday. Please re-assign if action is required before then. Thanks!

melvin-bot · 2023-06-30T20:09:11Z

@Beamanator, @NicMendonca Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

NicMendonca · 2023-06-30T20:29:28Z

@Beamanator anything needed from me here yet?

Beamanator · 2023-07-03T12:09:44Z

@NicMendonca I think we should be able to get 1 external person to audit the app & see if all of the above reported console errors are fixed - if not, spin up a PR to fix them 👍

melvin-bot · 2023-07-04T18:22:18Z

Job added to Upwork: https://www.upwork.com/jobs/~0144288ee67ea3d1ae

melvin-bot · 2023-07-04T18:22:21Z

Current assignee @NicMendonca is eligible for the External assigner, not assigning anyone new.

melvin-bot · 2023-07-04T18:22:27Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

alexxxwork · 2023-07-06T01:51:38Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

We get some prop type warnings / errors

What is the root cause of that problem?

accountID should be of type Number and there are some places in code when it is of type String

What changes do you think we should make in order to solve the problem?

We should change propTypes to Number here

App/src/libs/PusherUtils.js

Line 33 in 40c494f

* @param {String} accountID

because this func uses accountID from Onyx here:

App/src/libs/actions/User.js

Lines 24 to 30 in 0e21bc5

    
           Onyx.connect({ 
        
               key: ONYXKEYS.SESSION, 
        
               callback: (val) => { 
        
                   currentUserAccountID = lodashGet(val, 'accountID', -1); 
        
                   currentEmail = lodashGet(val, 'email', ''); 
        
               }, 
        
           });

App/src/libs/actions/User.js

Lines 576 to 580 in 0e21bc5

    
           PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.MULTIPLE_EVENTS, currentUserAccountID, (pushJSON) => { 
        
               _.each(pushJSON, (multipleEvent) => { 
        
                   PusherUtils.triggerMultiEventHandler(multipleEvent.eventType, multipleEvent.data); 
        
               }); 
        
           });

and we should explicitly convert accountID to Number when it is passed as a string from route params:

App/src/pages/ValidateLoginPage/index.js

Lines 43 to 59 in 0e21bc5

    
           useEffect(() => { 
        
               const login = lodashGet(props, 'credentials.login', null); 
        
               const accountID = lodashGet(props.route.params, 'accountID', ''); 
        
               const validateCode = lodashGet(props.route.params, 'validateCode', ''); 
        
               // A fresh session will not have credentials.login and user permission betas available. 
        
               // In that case, we directly allow users to go through password less flow 
        
               if (!login || canUsePasswordlessLogins) { 
        
                   if (lodashGet(props, 'session.authToken')) { 
        
                       // If already signed in, do not show the validate code if not on web, 
        
                       // because we don't want to block the user with the interstitial page. 
        
                       Navigation.goBack(false); 
        
                   } else { 
        
                       Session.signInWithValidateCodeAndNavigate(accountID, validateCode, preferredLocale); 
        
                   } 
        
               } else { 
        
                   User.validateLogin(accountID, validateCode);

accountID is string here:

App/src/pages/ValidateLoginPage/validateLinkPropTypes.js

Line 13 in 0e21bc5

accountID: PropTypes.string,

also here func params should be described properly and should be of type Number because selectedEmployees is an array of Number

App/src/pages/workspace/WorkspaceMembersPage.js

Lines 211 to 235 in 0e21bc5

    
               /** 
        
                * Add user from the selectedEmployees list 
        
                * 
        
                * @param {String} login 
        
                */ 
        
               const addUser = useCallback( 
        
                   (accountID) => { 
        
                       setSelectedEmployees((prevSelected) => [...prevSelected, accountID]); 
        
                       validateSelection(); 
        
                   }, 
        
                   [validateSelection], 
        
               ); 
        
               /** 
        
                * Remove user from the selectedEmployees list 
        
                * 
        
                * @param {String} login 
        
                */ 
        
               const removeUser = useCallback( 
        
                   (accountID) => { 
        
                       setSelectedEmployees((prevSelected) => _.without(prevSelected, accountID)); 
        
                       validateSelection(); 
        
                   }, 
        
                   [validateSelection], 
        
               );

and we should convert to Number here:

App/src/pages/workspace/WorkspaceMembersPage.js

Lines 252 to 254 in 0e21bc5

    
               removeUser(accountID); 
        
           } else { 
        
               addUser(accountID);

mananjadhav · 2023-07-06T20:28:53Z

@NicMendonca I think we should be able to get 1 external person to audit the app & see if all of the above reported console errors are fixed - if not, spin up a PR to fix them 👍

@Beamanator Is this like an investigation + fix? Because I can see proposals but I don't see any investigations?

Beamanator · 2023-07-07T11:26:17Z

@mananjadhav - yeah i think investigation just to the point of "does this proptype error still exist" & if it does, recommend the fix

alexxxwork · 2023-07-08T02:18:36Z

@Beamanator @mananjadhav All of the mentioned cases of console errors are now resolved

Beamanator · 2023-07-11T10:08:55Z

Okkk thanks @alexxxwork - if you see other cases please feel free to report in #expensify-bugs - for now let's close!

kbecciv · 2023-07-12T11:05:37Z

Hello @Beamanator! There is another console error when requesting money. Let me know if I need to open a separate ticket for that. Thank you

Beamanator · 2023-07-12T11:07:56Z

@kbecciv yes please create a different issue, this specific issue deals with a different type of console error (unrelated to what you found) 👍

kbecciv · 2023-07-12T11:24:20Z

Sound good @Beamanator

Beamanator added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 15, 2023

melvin-bot bot assigned NicMendonca Jun 15, 2023

Beamanator changed the title ~~[Onyx re-key] Prop type warnings~~ [Onyx re-key] Prop type warnings / errors Jun 15, 2023

Beamanator mentioned this issue Jun 15, 2023

[Tracking issue] Onyx re-key effort #20806

Closed

15 tasks

BeeMargarida mentioned this issue Jun 15, 2023

fix: select all workspace invite comparison #20826

Merged

56 tasks

melvin-bot bot added the Reviewing Has a PR in review label Jun 15, 2023

Beamanator self-assigned this Jun 19, 2023

alitoshmatov mentioned this issue Jun 22, 2023

DEV: Console error on Pressing invite button #21326

Closed

6 tasks

kushu7 mentioned this issue Jun 22, 2023

DEV: Console error when opening Pronouns page #21349

Closed

6 tasks

ayazhussain79 mentioned this issue Jun 23, 2023

DEV: Console error when openning user profile #21348

Closed

6 tasks

NicMendonca added the External Added to denote the issue can be worked on by a contributor label Jul 4, 2023

melvin-bot bot changed the title ~~[Onyx re-key] Prop type warnings / errors~~ [$1000] [Onyx re-key] Prop type warnings / errors Jul 4, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 4, 2023

melvin-bot bot assigned mananjadhav Jul 4, 2023

Beamanator closed this as completed Jul 11, 2023

kbecciv reopened this Jul 12, 2023

Beamanator closed this as completed Jul 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$1000] [Onyx re-key] Prop type warnings / errors #20802

[$1000] [Onyx re-key] Prop type warnings / errors #20802

Beamanator commented Jun 15, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

alitoshmatov commented Jun 15, 2023

NicMendonca commented Jun 16, 2023

Beamanator commented Jun 19, 2023

NicMendonca commented Jun 23, 2023

melvin-bot bot commented Jun 30, 2023

NicMendonca commented Jun 30, 2023

Beamanator commented Jul 3, 2023

melvin-bot bot commented Jul 4, 2023

melvin-bot bot commented Jul 4, 2023

melvin-bot bot commented Jul 4, 2023

alexxxwork commented Jul 6, 2023

mananjadhav commented Jul 6, 2023

Beamanator commented Jul 7, 2023

alexxxwork commented Jul 8, 2023

Beamanator commented Jul 11, 2023

kbecciv commented Jul 12, 2023

Beamanator commented Jul 12, 2023

kbecciv commented Jul 12, 2023

[$1000] [Onyx re-key] Prop type warnings / errors #20802

[$1000] [Onyx re-key] Prop type warnings / errors #20802

Comments

Beamanator commented Jun 15, 2023 • edited by melvin-bot bot Loading

melvin-bot bot commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

Bug0 Triage Checklist (Main S/O)

alitoshmatov commented Jun 15, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

NicMendonca commented Jun 16, 2023

Beamanator commented Jun 19, 2023

NicMendonca commented Jun 23, 2023

melvin-bot bot commented Jun 30, 2023

NicMendonca commented Jun 30, 2023

Beamanator commented Jul 3, 2023

melvin-bot bot commented Jul 4, 2023

melvin-bot bot commented Jul 4, 2023

melvin-bot bot commented Jul 4, 2023

alexxxwork commented Jul 6, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

mananjadhav commented Jul 6, 2023

Beamanator commented Jul 7, 2023

alexxxwork commented Jul 8, 2023

Beamanator commented Jul 11, 2023

kbecciv commented Jul 12, 2023

Beamanator commented Jul 12, 2023

kbecciv commented Jul 12, 2023

Beamanator commented Jun 15, 2023 •

edited by melvin-bot bot

Loading