Skip to content
This repository has been archived by the owner on Oct 14, 2024. It is now read-only.

NOISSUE - Repository Retrieve User By Name #84

Merged

Conversation

rodneyosodo
Copy link
Member

@rodneyosodo rodneyosodo commented Nov 28, 2023

What does this do?

Retrieve users by name from the database. This is a new feature. It enables the user to retrieve users by name. This is needed for search purposes

We are also retrieving the ID since name is not unique and email would leak sensitive information

Which issue(s) does this PR fix/relate to?

No issue

List any changes that modify/break current functionality

None

Have you included tests for your changes?

Yes

Did you document any new/modified functionality?

No

Notes

N/A

@rodneyosodo rodneyosodo force-pushed the 11-28-feat_add_retrieve_users_by_name branch 3 times, most recently from a1c203c to da304db Compare November 29, 2023 09:59
@rodneyosodo rodneyosodo force-pushed the 11-28-feat_add_retrieve_users_by_name branch 2 times, most recently from dcf34bd to 9042b1f Compare November 29, 2023 12:45
@rodneyosodo rodneyosodo force-pushed the 11-28-feat_add_retrieve_users_by_name branch 3 times, most recently from 1606def to c471101 Compare November 30, 2023 16:40
Copy link

codecov bot commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@44b537c). Click here to learn what that means.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #84   +/-   ##
=======================================
  Coverage        ?   56.77%           
=======================================
  Files           ?       91           
  Lines           ?     8817           
  Branches        ?        0           
=======================================
  Hits            ?     5006           
  Misses          ?     3445           
  Partials        ?      366           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@arvindh123 arvindh123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 257 to 267
if pm.Name != "" {
query = append(query, fmt.Sprintf("name ILIKE '%%%s%%'", pm.Name))
}
if pm.Identity != "" {
query = append(query, fmt.Sprintf("identity ILIKE '%%%s%%'", pm.Identity))
}

if len(query) > 0 {
emq = fmt.Sprintf("WHERE %s", strings.Join(query, " AND "))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe (for SQL injection)? Is there an option to build this using pgx lib?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this testing it is safe

image

Condition 5 doesn't happen since we wrap our search param with %:search%

@rodneyosodo rodneyosodo force-pushed the 11-28-feat_add_retrieve_users_by_name branch from f4d5e62 to 049a70d Compare December 4, 2023 09:11
Copy link
Contributor

@dborovcanin dborovcanin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing, please fix them.

rodneyosodo and others added 7 commits December 4, 2023 16:34
Retrieve users by name from the database. This is a new feature. It
enables the user to retrieve users by name.

Signed-off-by: rodneyosodo <[email protected]>
The changes made in this commit involve modifying the RetrieveNames function in the clients.go file. The function now retrieves client names by constructing a query. Additionally, test cases have been added to ensure the correct retrieval of clients with different parameters and to assert the expected response and error values.

Signed-off-by: Rodney Osodo <[email protected]>
When constructing a query, the function now checks if the order parameter is provided and if it is one of the allowed values (name, identity, created_at, updated_at). If so, the function adds an ORDER BY clause to the query. Additionally, if the direction parameter is provided and it is either "asc" or "desc", the function adds the direction to the query.

This change enables users to specify the order in which the query results should be returned and the direction of the ordering.

Signed-off-by: Rodney Osodo <[email protected]>
The fix updates the check to use the constants from the api package for clarity and consistency.

Signed-off-by: Rodney Osodo <[email protected]>
Added the ID field to the SELECT query in the RetrieveNames function in clients.go. This change allows the function to retrieve the ID along with the name of the clients.

Also modified the clients_test.go file to include the ID field in the test data for the RetrieveNames test.

Signed-off-by: Rodney Osodo <[email protected]>
This commit adds a new feature by including a username field in the client information. It also modifies the retrieval of client names and adds tests to prevent SQL injection. Additionally, it introduces functions for retrieving clients based on their identity and includes test functions for retrieving clients based on different criteria. The code also includes functions for setting parameters and finding clients based on specific queries.
This commit updates the SQL query in the RetrieveNames function in the users/postgres/clients.go file. The query now uses the '~' operator instead of 'ILIKE' for the name and identity conditions. This change improves the search functionality by allowing for more flexible matching.

Signed-off-by: Rodney Osodo <[email protected]>
@rodneyosodo rodneyosodo force-pushed the 11-28-feat_add_retrieve_users_by_name branch from fb274f1 to 975cd43 Compare December 4, 2023 13:34
@dborovcanin dborovcanin merged commit 38b50cb into absmach:main Dec 4, 2023
22 of 23 checks passed
@rodneyosodo rodneyosodo deleted the 11-28-feat_add_retrieve_users_by_name branch December 12, 2023 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants