Tasks#202
Conversation
❌ Deploy Preview for open-crowd-prod failed.
|
❌ Deploy Preview for open-devfounders failed.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ).then(({ rows }) => { | ||
| return rows.map((r) => ({ | ||
| id: r.id, | ||
| label: r.fullName, |
There was a problem hiding this comment.
Delete ,
| label: r.fullName, | |
| label: r.fullName |
| return rows.map((r) => ({ | ||
| ...r, | ||
| id: r.id, | ||
| label: r.displayName, |
There was a problem hiding this comment.
Delete ,
| label: r.displayName, | |
| label: r.displayName |
There was a problem hiding this comment.
Really some nice work here @gaspergrom! The UX of this feature looks and feels great 👍
Unfortunately, I'm a bit concerned with some of the architectural decisions that were made for this task module — let me elaborate:
Right now, when I create a task based on a suggestion, within the Tasks page, the frontend is making the following requests to the API:
- PUT request to update the value
- POST (query) request with
{ filter: { type: "suggested"} } - POST (query) request with
{ filter: { type: "regular", status: "in-progress"} } - POST (query) request with
{ filter: { {type: "regular", status: { eq: "in-progress" } } - POST (query) request with
{ filter: { type: "regular", status: "done" } }
Personally, this feels a bit too much since these 5 requests could probably be just one action (with one API call, the PUT request) that would receive the updated record from the API, and mutate/update the global state as we do in the rest of our modules.
By decentralizing a lot of the logic from the actions/store, using this event-driven architecture where we subscribe to store actions and move most of the logic to components, I think the code also got a lot more scattered, and harder to follow/manage.
At the same time, I'm also concerned that refactoring all these things might not be affordable right now, so I'll need to double-check with @joanreyero what's the best way to proceed.
Either way, thank you for this work! As I said, the experience of using the whole feature is really good 🙌
It would be really hard to put this into store since stuff isn't used on more places and for example in tasks we got 3 different filters where it takes tasks of all people in tenant, on home page we got just users tasks and suggested tasks in same component, on members page we display tasks related just to member so it was struggle to find solution to put this just into store since components didn't use same type of data |
mariobalca
left a comment
mariobalca
left a comment
There was a problem hiding this comment.
As discussed, will merge this for now 👍
Later on, if we feel that this approach represents a problem in terms of network usage, we'll revisit some of these decisions and refactor what will be needed.
Thanks again for all the hard work @gaspergrom 🙌
3 participants
Changes proposed ✍️
Screenshots (front-end changes only)
Checklist ✅
type:feature 🚀,type:enhancement ✨,type:bug 🐞, ortype:documentation 📜.frontend/.env.distbackend/.env.dist,backend/.env.dist.staging,backend/.env.dist.staging.