Fix return statement to immediately return data #563

kabaliisa · 2021-03-03T20:47:30Z

No description provided.

daltonfury42

Good one! Please send more PRs as you find things like these.

maaverik · 2021-03-04T10:14:53Z

simplq/src/api/auth.js

-    const response = await authedRequest;
-    return response.data;
+
+    return data;
  };


I was under the impression that these two versions are functionally equivalent. Could someone help me understand what I'm missing here 😕 ?

I was under the impression that these two versions are functionally equivalent. Could someone help me understand what I'm missing here ?

@maaverik using less variables reduces complexity of the algorithm. so instead of having 2 variables authedRequest and response we an have one.

Btw I'm getting this list of issues from here .I hope it's Ok

@maaverik using less variables reduces complexity of the algorithm. so instead of having 2 variables authedRequest and response we an have one.

Okay, got it. I thought there was something with promise returns that I was missing. Using fewer variables is a good point.

Btw I'm getting this list of issues from here .I hope it's Ok

Please go ahead, these are all things we should have addressed before.

@maaverik These are all code smells returned by Sonarqube.

@kabaliisa While you fix the issues, can you add a link to the corresponding sonarqube page in the PR description?

Btw I'm getting this list of issues from here .I hope it's Ok

These are great changes and we are also learning a lot from your PRs. :)

@maaverik These are all code smells returned by Sonarqube.

@kabaliisa While you fix the issues, can you add a link to the corresponding sonarqube page in the PR description?

Cool

This code smell was about return makeAuthedRequest and I am pretty sure it will still smell after this PR.

This change looks good to me. @mradenovic

const { data } = await ... return data;

is better than

authedRequest = ... response = await ... return response.data

The smell about return makeAuthedRequest, let me check and suppress it if it's not making sense.

* Remove amplify backend (#524) * Async actions for Queue Status and My Queues API calls (#522) * Add gitter chat badge (#530) * Pick api base url from config. (#525) * Remove unused backend env amplify (#532) * Add LoadingStatus Component (#531) * Add LoadingStatus Component Upgrade storybook to 6.1.17 Fix LoadingIndicator stories Add new Loading component Use Loading wrapper in Join page Separate Loading container from presentation Add Loading stories * Pass async action to LoadingStatus * Add PropTypes to LoadingStatus * Move auth0 out of async actions. (#535) Since an async action is called whithin a hook, it is guaranteed that it will be called within a component. It doesn't need to know anything about auth0. * Use storybook for components development (#533) * Add storybook esentials addon Use essential addons to develop components in isolation. See https://storybook.js.org/docs/react/essentials/introduction. * Add redux decorator to storybook All components will be wraped in store provider. Store state can be passed to container components thru components/story parameters. * Pass string instead of function to LoadingStatus Passing function just to avoid passing string, unnecessarily complicates the component and testing. * Add LoadingStatus stories * Fix bug in Loading component Loading component was returning children as a default option. * Add PropTypes to Loading component (#536) * Update CODEOWNERS (#538) * Cleaned up lots of ugly import statements (#541) * started cleaning up imports * Cleaned up more imprts * Final import cleanup * Use async actions for token status page (#526) * Tour guide implementation: Implemented the suggested changes. (#539) * Refactor join page (#542) * Add @storybook/addon-actions back to package.json The package is installed with @storybook/addon-essentials, so it doesn't affect package-lock.json. However, eslint warns that it should be listed as dependency. * Intercept dispatch in redux store decorator In order to develop components in isolation, all async calls should be intercepted and mocked. * Rename Join page file * Update relative paths * Remove history parameter from JoinPage History is no longer dependency of useEffects(), and it is passed to the HeaderSection, but never used. * Pass queueName from url params to HeaderSection Header section is visible while loading spinner. If queue exists, the name will be same as in url params. * Add JoinPage stories * Update QueueStats stories to storybook 6.x style * Reuse sotries data in JoinPage stories * Rename Join scss file * Rename Join form * Destrurcture props in JoinForm * Fix react wornings in JoinForm Callback chained to the promise was attempting to change the form after redirect, which was causing errors in the cosnole. * Add joining case to JoinPage stories This sotry desribes state after the user click Join button, before the request is fulfilled. * added star button (#540) * added star button * added line break at bottom * Refactor home page (#544) * Rename home files * Rename Layout files * Refactor loading handling in Layout * Load user queues when isAuthenticated changes User queues were loaded on every round trip to the home page. This should be done only once when the user state is known, and on every change in user status or queue list (create/delete/pause). This also gives an opportunity to reuse MyQueues component in the Admin page. * Create alias for CreateJoinForm CreateJoinForm sounds like a function that will create JoinForm. QueueForm is a more appropriate name. It implies it's a queue Form, without explaining what form actions are. * Add MyQueues stories * Fix code style issues with Prettier Co-authored-by: Lint Action <[email protected]> * Mock auth0 in storybook (#546) * Mock auth0 in storybook This allows components that depend on auth0 to be in developed in isolation. * Fix code style issues with Prettier Co-authored-by: Lint Action <[email protected]> * Changed createJoinForm to use redux actions (#553) * Changed createJoinForm to use redux actions * Modified LoadingStatus and used in JoinPage and CreateJoinForm * Fixed a bug in fn call and a type * Refactor admin page (#550) * Rename admin page file * Add getQueue() request creator * Add getSelectedQueue async action * Add selectedQueue slice * Add mock data for testing * Add Token stories * Remove token with dispatch * Add notifyToken() request creator * Add notifyToken() async action * Notify token with dispatch * Refactor NotifyButton * Extract CallButton * Redesign RemoveButton * Disable NotifyButton while notifyToken() is pending * Refactored delete queue (#551) * Refactored AddMember to use redux actions (#552) * Refactored AddMember * Fixed some imports * Added popup for adding to queues * Disable RemoveButton while dleteToken is pending * Refactor TaskList component * Extract AdminPage HeaderSection * Refactor AdminPage component * Fix: set timeoutId before clearing it Co-authored-by: Nithin <[email protected]> * Add anonymous UUID to unauthinticated API Calls (#554) * Add anonymous UUID to unauthinticated API Calls * Update jsdoc * Fix security vulnerabilities (#557) * Moved token and getUserQueues selectors to prevent useless side effects (#556) * Moved token selector to tokenlist * Moved getUserQueues dispatch to LandingPage to prevent unneeded popup on other page loads * Removed console.log * Moved getUserQueues back to Layout and changes the popup behaviour * Changed anchor tag redirects to use react router history * Add comments to layout * Fix code style issues with Prettier * Made utility function for delayed scrolling Co-authored-by: daltonfury42 <[email protected]> Co-authored-by: Lint Action <[email protected]> * Fixed broken queue details in admin page, removed unused hooks and components (#558) * Fixed queue details breaking for admin side panel * Made a QueueDetails component for admin * Changed loading condition * Removed LoadingIndicator * Removed useRequest and RequestFactory exports * Refactor status page (#562) * Replace makeAuthedRequest with hook Since no async actions use make authed request, remove it. * Rename Status page * Rename TokenStatusPage components * Add Token stories * Rename folder Status --> TokenStatus * Reuse TokenStatus stories * Fix useDeleteQueue to immediately return expression (#566) * Fix return statement to immediately expression (#564) * Fix return statement to immediately return data (#563) * Remove Stale Comments from ErrorHandler and others (#572) * Delete unused file aws-exports.js (#571) * Revert "Fix async actions to immediately return expression" (#569) * Fix: remove commented out code (#573) * smell issue#21- return authedRequest (#574) * Footer Greeting (#575) * Change: rename files as per sonarqube (#576) * Change: rename files (#565) * Added deployment process, remove Hall of Frame (#578) * Request refresh tokens from client side (#580) * Use Typeform for Contact Us (#581) * Handle token add/remove in selectedQueueSlice (#582) * Centralised request updates and failures (#583) * Prevent failed call with queueId undefined (#584) * Fix for loading component failure breaking user flow + reverting token remove button + package updates (#589) * Updated packages and modified loading component * Removed showing error message * Reverting token remove button * Missed adding this file * Added pointer cursor style * Show spiner on first load in token status page (#591) * Fix Shaky Sidepanel (#592) * Rename QueueStatus/QueueStats to QueueInfo (#593) * CSS Optimisations for small screen: Fix token and header sections (#596) Co-authored-by: Mischa Radenovic <[email protected]> Co-authored-by: Nithin <[email protected]> Co-authored-by: promarcussmith <[email protected]> Co-authored-by: Bhargava Prabu Reddy <[email protected]> Co-authored-by: Lint Action <[email protected]> Co-authored-by: hallelshohat <[email protected]> Co-authored-by: Solomon kabaliisa <[email protected]> Co-authored-by: Tarun Sharma <[email protected]> Co-authored-by: navaneeth-spotnana <[email protected]>

Fix return statement to immediately return data

cd59358

kabaliisa requested review from daltonfury42, maaverik and mradenovic as code owners March 3, 2021 20:47

daltonfury42 approved these changes Mar 4, 2021

View reviewed changes

maaverik reviewed Mar 4, 2021

View reviewed changes

daltonfury42 merged commit f00c95d into SimplQ:master Mar 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix return statement to immediately return data #563

Fix return statement to immediately return data #563

kabaliisa commented Mar 3, 2021

daltonfury42 left a comment

maaverik Mar 4, 2021

kabaliisa Mar 4, 2021

kabaliisa Mar 4, 2021 •

edited

Loading

maaverik Mar 4, 2021 •

edited

Loading

maaverik Mar 4, 2021 •

edited

Loading

daltonfury42 Mar 4, 2021 •

edited

Loading

daltonfury42 Mar 4, 2021 •

edited

Loading

kabaliisa Mar 4, 2021

mradenovic Mar 4, 2021

daltonfury42 Mar 6, 2021

Fix return statement to immediately return data #563

Fix return statement to immediately return data #563

Conversation

kabaliisa commented Mar 3, 2021

daltonfury42 left a comment

Choose a reason for hiding this comment

maaverik Mar 4, 2021

Choose a reason for hiding this comment

kabaliisa Mar 4, 2021

Choose a reason for hiding this comment

kabaliisa Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

maaverik Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

maaverik Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

daltonfury42 Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

daltonfury42 Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

kabaliisa Mar 4, 2021

Choose a reason for hiding this comment

mradenovic Mar 4, 2021

Choose a reason for hiding this comment

daltonfury42 Mar 6, 2021

Choose a reason for hiding this comment

kabaliisa Mar 4, 2021 •

edited

Loading

maaverik Mar 4, 2021 •

edited

Loading

maaverik Mar 4, 2021 •

edited

Loading

daltonfury42 Mar 4, 2021 •

edited

Loading

daltonfury42 Mar 4, 2021 •

edited

Loading