Could you update me about what is left for this project to be complete?
joseincandenza opened this issue · 3 comments
Hi,
@fairingrey apart from tests, could you briefly expose what is this project missing to be published as a complete realworld-example-app?
Thank you.
I'm currently studying your code, using it as a base for another app. I have experience using rust, but never used it to build a server, so my experience with diesel and actix is None.
I would like to help you to push this project to completion, if you are interested. I have some doubts, though, probably derived from the mentioned lack of experience.
For example, I don't understand the query logic that you implement when obtaining the DbExecutor Handle of GetArticles. Here you apply conditional logic to obtain the list filtered by the params pased by the user, but you make multiple calls to the database, and I don't get why:
if let Some(ref author_name) = msg.params.author {
let articles_by_author = articles::table
.inner_join(users::table)
.filter(users::username.eq(author_name))
.select(articles::id)
.load::<Uuid>(conn)?;
query = query.filter(articles::id.eq_any(articles_by_author));
}
Won't be easier/cheaper to just extend the query without touching the database? According to documentation, filter is comulative, so you can just chain filters conditionally:
if let Some(ref author_name) = msg.params.author {
query = query.filter(users::username.eq(author_name))
}
The query would be previously joined. And executed after all the possible filters.
Another point is the use of eq_any. According to the documentation, with postgres you should use eq(any()).
I have not yet started with tests, so I cannot know if this will work, but I thought you probably know better.
Thank you in advance.
Hey, sorry I haven't replied to this post in a while, since I've been busy with work and haven't found the time to do much open-source. Also the time I do get I focus more on Tide and other frameworks with async/await as first-class syntax. Especially as my work involves those things directly.
Yeah, throughout the code I've been meaning to clean up quite a few places where I'm inefficiently executing multiple queries through diesel. The explanations you're giving make sense, although I can't verify at the moment whether they will actually compile (and I have yet to re-review the original scheme for each of these tables).
Would you like to make those changes? I'd be happy to review and accept a PR with them.
As for what this project is missing, you are welcome to explore yourself what else this project requires or bump its' corresponding issue on the original realworld repo.