Skip to content
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

Update entities due to API change #16

Open
Dodje opened this issue Sep 4, 2024 · 12 comments
Open

Update entities due to API change #16

Dodje opened this issue Sep 4, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@Dodje
Copy link

Dodje commented Sep 4, 2024

Seems like there were some changes in API. In particular endpoint of the list: https://api.realworld.io/api/articles?limit=10
So there is the error while validation article schema: missing body prop..

I've tried to find correct doc like swagger or whatever to ensure DTO change, but I wasn't able to find it to exact api.realworld.io. And those docs that I've found say that body is expected to be.

I guess there might be some bug in api, so we can wait and check. Or maybe it was done on purpose..

@yurisldk
Copy link
Owner

yurisldk commented Sep 4, 2024

Hey, good catch. That's why we use contracts. According to this caution

GET /api/articles
GET /api/articles/feed

have been updated and do no longer return the body field.

image

@Dodje would you like to open a PR and fix the issue? I believe that deleting the body field in the schemas will solve the issue.

P. S. It looks like that realworld-docs is in the process of updating right now. @geromegrignon Do we have a centralized place where we can view all upcoming updates?

@yurisldk yurisldk added the bug Something isn't working label Sep 4, 2024
@Dodje
Copy link
Author

Dodje commented Sep 4, 2024

Yeah, I guess it will be possible for me to create a PR. But I believe there will be some other changes than deleting body prop.

It still exists in single article dto, so we should separate Article and ArticleListItem. I'll check it again more carefully today later or tomorrow.

@yurisldk yurisldk assigned yurisldk and Dodje and unassigned yurisldk Sep 4, 2024
@geromegrignon
Copy link

Hello, indeed, as unused for the purpose of the UI I recently removed it from the public API responses as a solution to reduce the server traffic (as I had to move to a paid one).

I'll double check the docs to ensure there is no mention left, I already removed it from the open API file.

@geromegrignon
Copy link

Currently there is no other plans to make changes due to the nature of the project: most implémentations are not maintained, making it hard to make breaking changes.

It might change in the future but I'm focusing on a spin-off focused on Angular for a more complex (and so more RealWorld) application as a way to get a playground I'll be be able to update with ease unlike this one.

I plan to add a proper documentation for anyone willing to create their own spin-off, like a one for React.

@yurisldk
Copy link
Owner

yurisldk commented Sep 4, 2024

@geromegrignon Sounds good, thank you for additional details.

@Dodje
Copy link
Author

Dodje commented Sep 5, 2024

@yurisldk Hi again :)

I went through the project today and found some other changes like "author.bio" -> optional for list item and nullable for single article.

At first I thought that It might be a good idea to fully separate DTO's and logic between list item and single item on the frontend. But it seems that there gonna be really lots of changes like creating/separating features for favorite-feed-article/favorite-article or creating middle model for both at once, query mutations changes and so on.

The simplest idea is to create ArticleListItemDto with optional body and convert to empty string for article entity, but It can help only in the short-term.. but it will make the app work at least

What do you think?

@Dodje
Copy link
Author

Dodje commented Sep 5, 2024

There is also missing updatedAt property from server DTO now. That eventually makes those single and list item article to be not reusable and to be totally separated.

@yurisldk
Copy link
Owner

yurisldk commented Sep 5, 2024

separate DTO's and logic between list item and single item

let's do this. We also must pay attention to article entite`s and feature`s optimistic update logic.

we can start with:

  1. separate DTO`s at src/shared/api/article

    • add new preview contract, for example ArticlePreviewDtoSchema
    • update ArticlesDtoSchema with the new preview contract
    • add new preview type, for example ArticlePreviewDto
  2. update article entity src/entities/article

    • add new preview contract, for example ArticlePreviewSchema
    • update ArticlesSchema with the new preview contract
    • add new preview type, for example ArticlePreview
    • add new transform method, for example transformArticlePreviewDtoToArticlePreview
    • update transformArticlesDtoToArticles with the new transform method
    • remove setArticleData method from ArticleQueries class

after that we have to take a look at features. Seems like we have to add additional mutation for article preview and cleanup article mutation optimistic update logic. At any case we have a good start point.

@Dodje
Copy link
Author

Dodje commented Sep 6, 2024

The first part is completed.
Need some time to meditate on ReactQuery docs :) Haven't worked with this tech yet

@Dodje
Copy link
Author

Dodje commented Sep 6, 2024

@geromegrignon I hope it's okay to ask you here since we've already begun the discussion.

I went throw the docs and I found some issues with it:

  1. Methods POST /api/articles/:slug/favorite and DELETE /api/articles/:slug/favorite should be reviewed since the separation of article DTOs. There is still expected Article in return, but now we have to operate with different DTOs for the same action and we can favorite/unfavorite Article Preview.

  2. The Article DTO link return 404.

@Dodje
Copy link
Author

Dodje commented Sep 6, 2024

@yurisldk I also have some questions to you if I may :)

I've found some schema named FavoriteArticleDtoSchema and I believe it shouldn't exist.
We work with exact Article anyway no need to create yet another contract.
If we need to separate api methods in shared layer which might be the case (negotiable/debatable) we can work with existing ArticleDTO as it is the same on the backend side.

@Dodje
Copy link
Author

Dodje commented Sep 6, 2024

I've pushed another commit 37d245c with favorite/unfavorite button changes.
Seems working fine except the thing that when I favorire the article or artice preview, there is a huge ping before data actually updates on the server. Maybe some cache issues...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants