Skip to content

Feat/version2.0 user requests#98

Merged
lfbrehm merged 8 commits intofeat/version2.0reworkfrom
feat/version2.0_user_requests
Aug 8, 2023
Merged

Feat/version2.0 user requests#98
lfbrehm merged 8 commits intofeat/version2.0reworkfrom
feat/version2.0_user_requests

Conversation

@lfbrehm
Copy link
Copy Markdown
Member

@lfbrehm lfbrehm commented Aug 4, 2023

This adds basic functionality for grpc-side user requests.

@lfbrehm lfbrehm requested review from St4NNi and das-Abroxas August 4, 2023 13:25
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 4, 2023

Codecov Report

Patch coverage: 2.54% and project coverage change: -1.84% ⚠️

Comparison is base (83a30b6) 21.23% compared to head (4b681ac) 19.40%.
Report is 1 commits behind head on feat/version2.0rework.

Additional details and impacted files
@@                    Coverage Diff                    @@
##           feat/version2.0rework      #98      +/-   ##
=========================================================
- Coverage                  21.23%   19.40%   -1.84%     
=========================================================
  Files                         43       48       +5     
  Lines                       3758     4268     +510     
=========================================================
+ Hits                         798      828      +30     
- Misses                      2960     3440     +480     
Files Changed Coverage Δ
src/caching/cache.rs 37.50% <0.00%> (-4.11%) ⬇️
src/grpc/users.rs 0.00% <0.00%> (ø)
src/middlelayer/token_db_handler.rs 0.00% <0.00%> (ø)
src/middlelayer/token_request_types.rs 0.00% <0.00%> (ø)
src/middlelayer/user_db_handler.rs 0.00% <0.00%> (ø)
src/middlelayer/user_request_types.rs 0.00% <0.00%> (ø)
src/utils/conversions.rs 0.00% <0.00%> (ø)
src/database/dsls/user_dsl.rs 53.19% <85.71%> (+1.50%) ⬆️

... and 1 file with indirect coverage changes

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

Copy link
Copy Markdown
Contributor

@das-Abroxas das-Abroxas left a comment

Choose a reason for hiding this comment

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

Will do 🤞

Copy link
Copy Markdown
Member

@St4NNi St4NNi left a comment

Choose a reason for hiding this comment

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

LGTM, Just some small hints!

Comment thread src/grpc/users.rs
);
let (user_id, user) = tonic_internal!(
self.database_handler.activate_user(request).await,
"Internal deactivate user error"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

c/p error

Comment thread src/grpc/users.rs
return_with_log!(response);
}

async fn get_api_tokens(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn´t this be easy to do as-well ? Since the tokens are now part of a user this should be quite straight forward.

Comment thread src/grpc/users.rs
return_with_log!(DeleteApiTokenResponse {});
}

async fn delete_api_tokens(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above


impl DatabaseHandler {
pub async fn create_token(&self) {
todo!()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On a database perspective this is straight, forward, create the token (with uuid) and add it to the jsonb attributes of the user

let user = User::get(id, client)
.await?
.ok_or_else(|| anyhow!("User not found"))?;
transaction.commit().await?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for a transaction here

let user = User::get(id, client)
.await?
.ok_or_else(|| anyhow!("User not found"))?;
transaction.commit().await?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Transaction is useless here

let user = User::get(user_id, client)
.await?
.ok_or_else(|| anyhow!("User not found"))?;
transaction.commit().await?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See above

let user = User::get(user_id, client)
.await?
.ok_or_else(|| anyhow!("User not found"))?;
transaction.commit().await?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See above

@lfbrehm lfbrehm merged commit 4b681ac into feat/version2.0rework Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants