added user ticket history #92
Conversation
|
@PoulavBhowmick03 Pls fix the conflict |
PoulavBhowmick03
force-pushed
the
support/ticket_history
branch
from
4060849 to
53b40c7
Compare
|
Hi @PoulavBhowmick03 You should run |
|
Hi @PoulavBhowmick03 are you still working on this? |
PoulavBhowmick03
force-pushed
the
support/ticket_history
branch
from
53b40c7 to
22287b3
Compare
|
@Abeeujah because of a previous merge, and a merge conflict, i was unable to run the sql migration, can you please check that out? it was giving me errors |
|
If you've ran a migration before, you might want to destroy the created database or maybe change the database name locally; This is another reason why Containers were recommended in the readme, it would help you setup and tear down databases easily. @PoulavBhowmick03 |
PoulavBhowmick03
force-pushed
the
support/ticket_history
branch
from
22287b3 to
2ea72dc
Compare
There was a problem hiding this comment.
While taking a step towards implementing a solution, I think it can be perfected from here on, From my review, it seems you were not so clear on the module system and visibility scopes in Rust, here's the Rust programming language reference to that. Rust Modules
Please when done with your implementation, in this order locally,
cargo check
cargo fmt
cargo clippy
cargo test
This uses the rust toolchain to prepare your code to the Rust Programming language specification and linting rules.
|
|
||
| #[derive(Debug, serde::Deserialize)] | ||
| struct TicketHistoryParams { | ||
| wallet: String, |
There was a problem hiding this comment.
Missing the pub keyword here
| AppState, Error, Result, | ||
| http::support_ticket::{ListTicketsQuery, SupportTicket}, | ||
| http::support_ticket::{ListTicketsQuery, SupportTicket, TicketHistoryParams}, | ||
| }; |
There was a problem hiding this comment.
As a result of the TicketHistoryParams not being made public, by omitting the pub keyword in its definition in src/http/support_ticket/domain.rs this particular import cannot be resolved.
Please fix the struct declaration and proceed.
| Json, Router, | ||
| extract::{Query, State}, | ||
| http::StatusCode, | ||
| routing::{get, post}, |
There was a problem hiding this comment.
A couple unused imports here, the linter flags warnings as an error, ensuring that they are resolved; Unused variables or imports fall in the warnings category.
If you import a type or function and you later on do not use it, you might want to remove the imports to avoid polluting your modules.
| State(state): State<AppState>, | ||
| Query(params): Query<TicketHistoryParams>, | ||
| ) -> Result<Json<Vec<super::types::SupportTicket>>, StatusCode> { | ||
| let wallet = params.wallet.trim(); |
There was a problem hiding this comment.
To be fair, There's no types module in the immediate super of your list_ticket module. so even if `SupportTicket exists, and is a type being used in this codebase, the Rust Compiler has no way to find and link it.
Please could you look more closely into minor subtle errors as this?
A simple cargo check helps you spot this locally.
| "#; | ||
|
|
||
| let tickets = match sqlx::query_as::<_, super::types::SupportTicket>(query) | ||
| .bind(wallet) |
There was a problem hiding this comment.
Same error message here about the super; types.
|
|
||
| let page = params.page.unwrap_or(1).max(1); | ||
| let limit = params.limit.unwrap_or(20).min(100); | ||
| let offset = (page - 1) * limit; |
There was a problem hiding this comment.
you might want to keep the unwrap_or(10) at 10, and it shouldn't exceed 20 per query min(20)
| }; | ||
|
|
||
| Ok(Json(tickets)) | ||
| } |
There was a problem hiding this comment.
When you've fixed every errors and warnings in this file, you might want to format your code via the rust formatter, cargo fmt gets you going.
| post(resolve_ticket::resolve_ticket_handler), | ||
| ) | ||
| .route("/tickets", get(list_tickets::list_tickets_handler)) | ||
| .route("/ticket_history", get(list_tickets::ticket_history_handler)) |
There was a problem hiding this comment.
Similar issue as encountered above, the ticket_history_handler is private, unless specified to be public by adding the pub keyword at the function's declaration, it won't be visible to an external module.
| .expect("Failed to insert ticket"); | ||
| } | ||
|
|
||
| let req = Request::get(&format!("/ticket_history/{wallet}?page=1&page_size=3")) |
There was a problem hiding this comment.
Did you mean to say limit instead of page_size?
| assert_eq!(tickets[1]["subject"], "Sub 3"); | ||
| assert_eq!(tickets[2]["subject"], "Sub 2"); | ||
|
|
||
| let req = Request::get(&format!("/ticket_history/{wallet}?page=2&page_size=3")) |
There was a problem hiding this comment.
Same here, page_size || limit?
3 participants
Fixes #78