-
-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Adding new route for check if user is in whatsapp #231
base: main
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@RuanAyram has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a new feature for checking a user's WhatsApp status. The changes span multiple files across the application, adding a new endpoint for user verification, updating the backend service, creating new data structures, and implementing frontend components for user checks. The implementation allows users to verify if a specific phone number is registered on WhatsApp, with support for API calls, validation, and a user-friendly interface. Changes
Sequence DiagramsequenceDiagram
participant User
participant Frontend
participant API
participant UserService
participant WhatsApp
User->>Frontend: Initiate User Check
Frontend->>API: Send Check Request
API->>UserService: Validate Request
UserService->>WhatsApp: Check User Status
WhatsApp-->>UserService: Return User Status
UserService-->>API: Prepare Response
API-->>Frontend: Return User Details
Frontend->>User: Display User Status
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (6)
src/validations/user_validation.go (1)
21-31
: Consider enhancing phone number validation.While the basic required field validation is implemented, consider adding more robust phone number validation:
- Format validation (E.164 format)
- Length checks
- Country code validation
Example enhancement:
func ValidateUserCheck(ctx context.Context, request domainUser.CheckRequest) error { err := validation.ValidateStructWithContext(ctx, &request, - validation.Field(&request.Phone, validation.Required), + validation.Field(&request.Phone, + validation.Required, + validation.Match(regexp.MustCompile(`^\+[1-9]\d{1,14}$`)). + Error("phone number must be in E.164 format"), + validation.Length(8, 15). + Error("phone number length must be between 8 and 15 digits"), + ), ) if err != nil { return pkgError.ValidationError(err.Error()) } return nil }src/internal/rest/user.go (1)
44-60
: Update success message to reflect the actual operation.The success message should be "Success check user" instead of "Success get user info" to accurately reflect the operation being performed.
- Message: "Success get user info", + Message: "Success check user",src/services/user.go (2)
89-95
: Use String() method instead of fmt.Sprintf.Replace
fmt.Sprintf("%s", item.JID)
withitem.JID.String()
for more idiomatic Go code.if item.VerifiedName != nil { - var msg = domainUser.CheckResponseData{Query: item.Query, IsInWhatsapp: item.IsIn, JID: fmt.Sprintf("%s", item.JID), VerifiedName: item.VerifiedName.Details.GetVerifiedName()} + var msg = domainUser.CheckResponseData{Query: item.Query, IsInWhatsapp: item.IsIn, JID: item.JID.String(), VerifiedName: item.VerifiedName.Details.GetVerifiedName()} uc.Users = append(uc.Users, msg) } else { - var msg = domainUser.CheckResponseData{Query: item.Query, IsInWhatsapp: item.IsIn, JID: fmt.Sprintf("%s", item.JID), VerifiedName: ""} + var msg = domainUser.CheckResponseData{Query: item.Query, IsInWhatsapp: item.IsIn, JID: item.JID.String(), VerifiedName: ""} uc.Users = append(uc.Users, msg) }🧰 Tools
🪛 golangci-lint (1.62.2)
90-90: S1025: should use String() instead of fmt.Sprintf
(gosimple)
93-93: S1025: should use String() instead of fmt.Sprintf
(gosimple)
98-101
: Remove commented-out code.Remove the commented line
// response := string(responseJson)
as it's not being used.response.Data = append(response.Data, uc.Users[0]) - // response := string(responseJson)
src/views/components/AccountUserCheck.js (2)
50-59
: Refactor error handling to avoid code duplication.The error handling code duplicates the null assignment of
verified_name
andjid
. Consider moving these assignments to a single location.} catch (error) { + this.resetUserInfo(); if (error.response) { - this.verified_name = null; - this.jid = null; throw new Error(error.response.data.message); } - this.verified_name = null; - this.jid = null; throw new Error(error.message); } finally { this.loading = false; } }, + resetUserInfo() { + this.verified_name = null; + this.jid = null; + },
100-113
: Refactor template to avoid code duplication.The template duplicates the user information display logic. Consider extracting it into a reusable template section.
- <div v-if="is_in_whatsapp != null" class="center"> - <ol> - <li>Name: {{ verified_name }}</li> - <li>JID: {{ jid }}</li> - </ol> - </div> - <div v-else class="center"> - <div v-if="code == 'INVALID_JID'" class="center"> - <ol> - <li>Name: {{ verified_name }}</li> - <li>JID: {{ jid }}</li> - </ol> - </div> - </div> + <div v-if="is_in_whatsapp != null || code == 'INVALID_JID'" class="center"> + <ol> + <li>Name: {{ verified_name }}</li> + <li>JID: {{ jid }}</li> + </ol> + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore
(1 hunks)readme.md
(3 hunks)src/domains/user/account.go
(2 hunks)src/domains/user/user.go
(1 hunks)src/internal/rest/user.go
(2 hunks)src/services/user.go
(1 hunks)src/validations/user_validation.go
(1 hunks)src/views/components/AccountUserCheck.js
(1 hunks)src/views/components/generic/FormCheckUserRecipient.js
(1 hunks)src/views/index.html
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 Markdownlint (0.37.0)
readme.md
108-108: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
110-110: null
Bare URL used
(MD034, no-bare-urls)
112-112: null
Bare URL used
(MD034, no-bare-urls)
🪛 Gitleaks (8.21.2)
readme.md
110-111:
(curl-auth-header)
🪛 golangci-lint (1.62.2)
src/services/user.go
90-90: S1025: should use String() instead of fmt.Sprintf
(gosimple)
93-93: S1025: should use String() instead of fmt.Sprintf
(gosimple)
81-81: ineffectual assignment to jids
(ineffassign)
🔇 Additional comments (5)
src/domains/user/user.go (1)
9-9
: LGTM! Interface method signature is well-defined.The new
Check
method follows the established pattern of the interface, with appropriate request/response types and error handling.src/internal/rest/user.go (1)
17-17
: LGTM!The new route "/user/check" is properly registered in the InitRestUser function.
src/views/index.html (1)
104-104
: LGTM!The AccountUserCheck component is properly integrated into the application:
- Component is imported correctly
- Added to the Account section in the template
- Registered in the Vue app's components list
Also applies to: 162-162, 203-203
readme.md (2)
122-122
: LGTM!The feature table entry is well-formatted and consistent with the existing documentation.
176-176
: LGTM!The screenshot addition follows the established pattern and provides good visual documentation of the new feature.
remove unused lines Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/services/user.go (1)
75-78
: Consider adding timeout handling and response validation.The implementation could benefit from:
- Timeout handling similar to the
Avatar
method to prevent indefinite waits- Response validation before processing to ensure
resp
is not nil or emptyConsider applying this pattern:
+ ctx, cancel := context.WithTimeout(ctx, 2*time.Second) + defer cancel() + + respChan := make(chan []whatsmeow.IsOnWhatsAppResponse) + errChan := make(chan error) + + go func() { resp, err := service.WaCli.IsOnWhatsApp([]string{request.Phone}) - if err != nil { - return response, err + if err != nil { + errChan <- err + return } + if len(resp) == 0 { + errChan <- errors.New("empty response from WhatsApp") + return + } + respChan <- resp + }() + + select { + case <-ctx.Done(): + return response, pkgError.ContextError("timeout checking WhatsApp status") + case err := <-errChan: + return response, err + case resp := <-respChan:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/user.go
(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
src/services/user.go
83-83: S1025: should use String() instead of fmt.Sprintf
(gosimple)
86-86: S1025: should use String() instead of fmt.Sprintf
(gosimple)
🔇 Additional comments (1)
src/services/user.go (1)
69-73
: LGTM! Good validation practices.The implementation follows good practices with early validation return pattern and clear separation of concerns.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@RuanAyram Thanks for your effort, I will review this PR after new version (v5.0.0) has been released, to avoid conflict |
Thanks for the answer @aldinokemal . So I'll wait for 5.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently IsOnWhatsapp function not working with 10 digit phone number
showErrorInfo(err) | ||
} | ||
}, | ||
async submitApi() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add is validate method like existing else component?
@@ -0,0 +1,50 @@ | |||
export default { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this generic
Context
Changes
and others files.