feat(platform): Create ui link for resend otp #489

Prakhargarg-2010196 · 2024-10-14T10:11:21Z

User description

Description

Adding the link to platform ui for resending otp

Fixes #479

Screenshots of relevant screens

Developer's checklist

My PR follows the style guidelines of this project
I have performed a self-check on my work

If changes are made in the code:

I have followed the coding guidelines
My changes in code generate no new warnings
I have added relevant screenshots in my PR
There are no UI/UX issues(I suppose)

PR Type

Enhancement

Description

Implemented the resend OTP functionality in the authentication page to improve user experience.
Added a new function handleResendOtp to manage the resend OTP process, including API call and error handling.
Introduced a loading state and UI feedback for the resend OTP action, including success and error notifications using toast.
Enhanced the user interface with a "Resend OTP" button and loading indicator to guide users through the process.

Changes walkthrough 📝

Relevant files

Enhancement

page.tsx `Implement resend OTP functionality in authentication page` apps/platform/src/app/auth/otp/page.tsx Added state management for resend OTP loading status. Implemented `handleResendOtp` function to handle OTP resend logic. Added UI elements for the resend OTP button with loading indicator. Integrated toast notifications for success and error scenarios.	+45/-1

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

codiumai-pr-agent-free · 2024-10-14T10:11:47Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 479 - Partially compliant Fully compliant requirements: Implement UI for resend OTP flow in apps/platform/src/app/auth/otp/page.tsx Call {BASE_URL}/api/auth/send-otp/:email endpoint when resend OTP button is clicked Not compliant requirements: Follow Figma UI design for resend OTP flow
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Error Handling The error handling in the `handleResendOtp` function could be improved. Currently, it only checks for a 429 status code, but other error scenarios are not specifically handled. UI Consistency The implementation of the resend OTP UI might not fully match the Figma design as mentioned in the ticket. A thorough comparison with the provided Figma link is needed.

codiumai-pr-agent-free · 2024-10-14T10:12:19Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Improve code organization by extracting the OTP resend logic into a custom hook Consider extracting the OTP resend logic into a separate custom hook or utility function to improve code organization and reusability. apps/platform/src/app/auth/otp/page.tsx [92-113] -const handleResendOtp = async (userEmail: string): Promise<void> => { - try { - setIsLoadingRefresh(true) - const response = await fetch( - `${process.env.NEXT_PUBLIC_BACKEND_URL}/api/auth/resend-otp/${encodeURIComponent(userEmail)}`, - { - method: 'POST' +const useResendOtp = () => { + const [isLoadingRefresh, setIsLoadingRefresh] = useState(false); + + const handleResendOtp = async (userEmail: string): Promise<void> => { + try { + setIsLoadingRefresh(true); + const response = await fetch( + `${process.env.NEXT_PUBLIC_BACKEND_URL}/api/auth/resend-otp/${encodeURIComponent(userEmail)}`, + { method: 'POST' } + ); + if (response.status === 429) { + toast.error("Couldn't send OTP, too many requests"); + } else if (response.ok) { + toast.success('OTP successfully sent to your email'); } - ) - if (response.status === 429) { - toast.error("Couldn't send OTP, too many requests") - setIsLoadingRefresh(false) - } else if (response.ok) - toast.success('OTP successfully sent to your email') - setIsLoadingRefresh(false) - } catch (error) { - // eslint-disable-next-line no-console -- we need to log the error - console.error(`Failed to send OTP: ${error}`) - toast.error("Couldn't send OTP .") - setIsLoadingRefresh(false) - } -} + } catch (error) { + console.error('Failed to send OTP:', error); + toast.error("Couldn't send OTP. Please try again later."); + } finally { + setIsLoadingRefresh(false); + } + }; + return { handleResendOtp, isLoadingRefresh }; +}; + +// In the component: +const { handleResendOtp, isLoadingRefresh } = useResendOtp(); + Apply this suggestion Suggestion importance[1-10]: 7 Why: Extracting the OTP resend logic into a custom hook can improve code organization and reusability. This change enhances maintainability by separating concerns and making the codebase more modular.	7
Maintainability	Improve code readability by using more descriptive variable names Consider using a more descriptive variable name for the OTP input state, such as `otpInput` or `enteredOtp`, to improve code readability. apps/platform/src/app/auth/otp/page.tsx [26] -const [otp, setOtp] = useState<string>('') +const [otpInput, setOtpInput] = useState<string>('') Apply this suggestion Suggestion importance[1-10]: 6 Why: Using a more descriptive variable name for the OTP input state can improve code readability. This is a minor enhancement but contributes to better understanding and maintainability of the code.	6
Maintainability	Improve code maintainability by using constants for configuration values Consider using a constant for the maximum number of OTP resend attempts to make the code more maintainable and easier to update in the future. apps/platform/src/app/auth/otp/page.tsx [101-106] +const MAX_OTP_RESEND_ATTEMPTS = 3; // Define this constant at the top of the file + if (response.status === 429) { - toast.error("Couldn't send OTP, too many requests") + toast.error(`Couldn't send OTP, maximum ${MAX_OTP_RESEND_ATTEMPTS} attempts reached`) setIsLoadingRefresh(false) -} else if (response.ok) +} else if (response.ok) { toast.success('OTP successfully sent to your email') +} setIsLoadingRefresh(false) Apply this suggestion Suggestion importance[1-10]: 5 Why: Introducing a constant for the maximum number of OTP resend attempts can improve maintainability by centralizing configuration values. However, the suggestion assumes a specific logic that is not present in the current code, which slightly reduces its relevance.	5

💡 Need additional feedback ? start a PR chat

kriptonian1

Everything looks good, just you need to organise the API call in the right file

kriptonian1 · 2024-10-15T11:22:57Z

apps/platform/src/app/auth/otp/page.tsx

+  const handleResendOtp = async (userEmail: string): Promise<void> => {
+    try {
+      setIsLoadingRefresh(true)
+      const response = await fetch(
+        `${process.env.NEXT_PUBLIC_BACKEND_URL}/api/auth/resend-otp/${encodeURIComponent(userEmail)}`,
+        {
+          method: 'POST'
+        }
+      )
+      if (response.status === 429) {
+        toast.error("Couldn't send OTP, too many requests")
+        setIsLoadingRefresh(false)
+      } else if (response.ok)
+        toast.success('OTP successfully sent to your email')
+      setIsLoadingRefresh(false)
+    } catch (error) {
+      // eslint-disable-next-line no-console -- we need to log the error
+      console.error(`Failed to send OTP: ${error}`)
+      toast.error("Couldn't send OTP .")
+      setIsLoadingRefresh(false)
+    }
+  }


Can you organize the API logic in apps/platform/src/lib/api-functions rest looks good

Don't use your own calls to the API. Rather use this:

keyshade/packages/api-client/src/controllers/user.ts

Lines 55 to 65 in 5884833

async resendEmailChangeOTP(

request: ResendEmailChangeOTPRequest,

headers?: Record<string, string>

): Promise<ClientResponse<ResendEmailChangeOTPResponse>> {

const response = await this.apiClient.post(

`./api/user/resend-email-change-otp`,

request,

headers

)

return await parseResponse<ResendEmailChangeOTPResponse>(response)

}

You can check the usage of these in any of the CLI files: https://github.com/keyshade-xyz/keyshade/tree/develop/apps/cli/src/commands

resendEmailChangeOTP

I got the point of the controller code from api-client but not the second thing you said

You can check the usage of these in any of the CLI files: https://github.com/keyshade-xyz/keyshade/tree/develop/apps/cli/src/commands

Oh, I meant that you can check any of the files in the commands directory to understand how to use these controllers.

So i have to write a new controller for auth and utilise as a controller instance if I am getting it right?

Is there a controller instance file similar instance to https://github.com/keyshade-xyz/keyshade/blob/develop/apps/cli/src/util/controller-instance.ts or I need to create one for the frontend of platform?

Prakhargarg-2010196 · 2024-10-15T11:39:08Z

Everything looks good, just you need to organise the API call in the right file

Can you please point that file if possible it would be helpful thanks.

kriptonian1 · 2024-10-15T13:35:51Z

Everything looks good, just you need to organise the API call in the right file

Can you please point that file if possible it would be helpful thanks.

I have mentioned the file in the review

rajdip-b · 2024-10-15T16:42:33Z

Everything looks good, just you need to organise the API call in the right file

Can you please point that file if possible it would be helpful thanks.

I have mentioned the file in the review

We shouldn't be using a separate api function folder, we already have a dedicated package for that.

kriptonian1 · 2024-10-15T17:06:39Z

Everything looks good, just you need to organise the API call in the right file

Can you please point that file if possible it would be helpful thanks.

I have mentioned the file in the review

We shouldn't be using a separate api function folder, we already have a dedicated package for that.

Looks like I need to migrate some stuffs

rajdip-b · 2024-10-24T11:39:37Z

@kriptonian1 @Prakhargarg-2010196 lmk when this is ready

Prakhargarg-2010196 · 2024-10-24T13:52:09Z

@kriptonian1 @Prakhargarg-2010196 lmk when this is ready

I am working on the changes but was bit confused
Two api-client.ts files present one in the apps/platform/src/lib/api-client.ts and another where you suggested in packages
So i need to use which one @kriptonian1? And if so shall the api-client in platform be deleted ?

rajdip-b · 2024-10-24T16:19:57Z

Let the api-client.ts file be. You would need to use the one from api-client package.

rajdip-b · 2024-10-29T17:08:56Z

@Prakhargarg-2010196 can you please configure ESLint on your device to use the config from keyshade? There are linting errors

apps/platform/src/app/auth/otp/page.tsx

Prakhargarg-2010196 · 2024-10-29T18:34:28Z

@Prakhargarg-2010196 can you please configure ESLint on your device to use the config from keyshade? There are linting errors

That's what I was saying the errors are not being thrown locally when i run pnpm run lint doesn't shows any errors on local.

Can you tell me some way to know which config is being consumed

rajdip-b · 2024-10-30T04:50:59Z

Okay got it. I'll switch to your PR and check it manually

- Created new controller instance for better management of API interactions. - Added authentication controller in API client for handling OTP-related requests. - Introduced type definitions for authentication to improve type safety. - Renamed index types definition for consistency.

rajdip-b · 2024-10-30T05:36:01Z

@kriptonian1 if you can lend me a hand then it would be awesome

rajdip-b · 2024-11-02T11:34:26Z

@Prakhargarg-2010196 I have relaxed the restrictions on ESLint

rajdip-b · 2024-11-02T11:37:53Z

You can ignore the lint failure, that's probably false.

Prakhargarg-2010196 · 2024-11-02T15:39:11Z

You can ignore the lint failure, that's probably false.

Then merge can be done right ?

rajdip-b

LGTM

rajdip-b · 2024-11-02T17:38:19Z

You can ignore the lint failure, that's probably false.

Then merge can be done right ?
Once @kriptonian1 drops his review, we can

## [2.7.0](v2.6.0...v2.7.0) (2024-11-05) ### 🚀 Features * **cli:** Add functionality to operate on Variables ([#514](#514)) ([32d93e6](32d93e6)) * **platform:** Create ui link for resend otp ([#489](#489)) ([46eb5c5](46eb5c5)) ### 🐛 Bug Fixes * **api,api-client:** Add environmentSlug in multiple places across the [secure] module ([#509](#509)) ([ee58f07](ee58f07)) * **cli:** Removed unnecessary console log in [secure]s ([#515](#515)) ([9403cc4](9403cc4)) ### 🔧 Miscellaneous Chores * Fixed lint issues ([835397a](835397a)) * Minor housekeeping ([922bf31](922bf31)) * Update eslint ([c583718](c583718)) * Update eslint ([7c0c596](7c0c596)) * Update pnpx commands to pnpm dlx ([#511](#511)) ([534a231](534a231))

rajdip-b · 2024-11-05T10:02:40Z

🎉 This PR is included in version 2.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Prakhargarg-2010196 requested a review from kriptonian1 as a code owner October 14, 2024 10:11

codiumai-pr-agent-free bot added enhancement Review effort [1-5]: 2 labels Oct 14, 2024

Prakhargarg-2010196 changed the title ~~Create ui link for resend otp~~ feat(platform):Create ui link for resend otp Oct 14, 2024

Prakhargarg-2010196 changed the title ~~feat(platform):Create ui link for resend otp~~ feat: Create ui link for resend otp Oct 14, 2024

rajdip-b changed the title ~~feat: Create ui link for resend otp~~ feat(platform): Create ui link for resend otp Oct 14, 2024

kriptonian1 added the hacktoberfest-accepted Your PR has this = Congrats! label Oct 15, 2024

kriptonian1 requested changes Oct 15, 2024

View reviewed changes

rajdip-b force-pushed the PI-resend-otp-frontend branch from 32ea02b to 017828c Compare October 15, 2024 13:24

rajdip-b force-pushed the PI-resend-otp-frontend branch from 017828c to e908b20 Compare October 24, 2024 11:39

rajdip-b reviewed Oct 29, 2024

View reviewed changes

apps/platform/src/app/auth/otp/page.tsx Outdated Show resolved Hide resolved

Prakhargarg-2010196 added 3 commits October 30, 2024 10:21

feat(platform):Create ui link for resend otp

f76ef3b

fix(platform):Remove unnecessary types imports

4b7d912

rajdip-b force-pushed the PI-resend-otp-frontend branch from 150d88f to 4b7d912 Compare October 30, 2024 04:51

update ESLint rules for platform

a610bcb

rajdip-b approved these changes Nov 2, 2024

View reviewed changes

kriptonian1 approved these changes Nov 2, 2024

View reviewed changes

rajdip-b merged commit 46eb5c5 into keyshade-xyz:develop Nov 3, 2024
4 of 5 checks passed

Prakhargarg-2010196 mentioned this pull request Nov 4, 2024

Resend OTP feature #325

Closed

2 tasks

rajdip-b added the released label Nov 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(platform): Create ui link for resend otp #489

feat(platform): Create ui link for resend otp #489

Prakhargarg-2010196 commented Oct 14, 2024 •

edited by codiumai-pr-agent-free bot

Loading

codiumai-pr-agent-free bot commented Oct 14, 2024

codiumai-pr-agent-free bot commented Oct 14, 2024 •

edited

Loading

kriptonian1 left a comment

kriptonian1 Oct 15, 2024

rajdip-b Oct 15, 2024

Prakhargarg-2010196 Oct 15, 2024

rajdip-b Oct 15, 2024

Prakhargarg-2010196 Oct 16, 2024

Prakhargarg-2010196 commented Oct 15, 2024

kriptonian1 commented Oct 15, 2024

rajdip-b commented Oct 15, 2024

kriptonian1 commented Oct 15, 2024

rajdip-b commented Oct 24, 2024

Prakhargarg-2010196 commented Oct 24, 2024

rajdip-b commented Oct 24, 2024

rajdip-b commented Oct 29, 2024

Prakhargarg-2010196 commented Oct 29, 2024

rajdip-b commented Oct 30, 2024

rajdip-b commented Oct 30, 2024

rajdip-b commented Nov 2, 2024

rajdip-b commented Nov 2, 2024

Prakhargarg-2010196 commented Nov 2, 2024

rajdip-b left a comment

rajdip-b commented Nov 2, 2024

rajdip-b commented Nov 5, 2024

	async resendEmailChangeOTP(
	request: ResendEmailChangeOTPRequest,
	headers?: Record<string, string>
	): Promise<ClientResponse<ResendEmailChangeOTPResponse>> {
	const response = await this.apiClient.post(
	`./api/user/resend-email-change-otp`,
	request,
	headers
	)
	return await parseResponse<ResendEmailChangeOTPResponse>(response)
	}

feat(platform): Create ui link for resend otp #489

feat(platform): Create ui link for resend otp #489

Conversation

Prakhargarg-2010196 commented Oct 14, 2024 • edited by codiumai-pr-agent-free bot Loading

User description

Description

Screenshots of relevant screens

Developer's checklist

PR Type

Description

Changes walkthrough 📝

codiumai-pr-agent-free bot commented Oct 14, 2024

PR Reviewer Guide 🔍

codiumai-pr-agent-free bot commented Oct 14, 2024 • edited Loading

PR Code Suggestions ✨

kriptonian1 left a comment

Choose a reason for hiding this comment

kriptonian1 Oct 15, 2024

Choose a reason for hiding this comment

rajdip-b Oct 15, 2024

Choose a reason for hiding this comment

Prakhargarg-2010196 Oct 15, 2024

Choose a reason for hiding this comment

rajdip-b Oct 15, 2024

Choose a reason for hiding this comment

Prakhargarg-2010196 Oct 16, 2024

Choose a reason for hiding this comment

Prakhargarg-2010196 commented Oct 15, 2024

kriptonian1 commented Oct 15, 2024

rajdip-b commented Oct 15, 2024

kriptonian1 commented Oct 15, 2024

rajdip-b commented Oct 24, 2024

Prakhargarg-2010196 commented Oct 24, 2024

rajdip-b commented Oct 24, 2024

rajdip-b commented Oct 29, 2024

Prakhargarg-2010196 commented Oct 29, 2024

rajdip-b commented Oct 30, 2024

rajdip-b commented Oct 30, 2024

rajdip-b commented Nov 2, 2024

rajdip-b commented Nov 2, 2024

Prakhargarg-2010196 commented Nov 2, 2024

rajdip-b left a comment

Choose a reason for hiding this comment

rajdip-b commented Nov 2, 2024

rajdip-b commented Nov 5, 2024

Prakhargarg-2010196 commented Oct 14, 2024 •

edited by codiumai-pr-agent-free bot

Loading

codiumai-pr-agent-free bot commented Oct 14, 2024 •

edited

Loading