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

Refactor Follows API #510

Closed
MagdaN opened this issue Nov 24, 2016 · 5 comments
Closed

Refactor Follows API #510

MagdaN opened this issue Nov 24, 2016 · 5 comments

Comments

@MagdaN
Copy link
Contributor

MagdaN commented Nov 24, 2016

The Follows-API is working nicely but I think it is implemented rather complicated:

  1. not restful in the sense of the DRF
  2. use PUT to create (with a custom mixin)
  3. error on page with no follow #507

This makes it hard to understand the code to others and also is implemented differently then all the other API-Endpoints

Instead I would suggest to just have a Follows list, that allows POST. When the Follow-Button in the React-Component is clicked, it sends a POST-Request to the Followlist (includes the project and the enabledflag) -> there the create function checks if an entry for a current user already exists, if not it creates one, if yes it updates the existing one.

The initial value is added to the react component when it is called in the template tag, no GET request is needed.

@slomo what do you think? Would that work or did I miss something?

@slomo
Copy link
Contributor

slomo commented Nov 28, 2016

I see your point and but decided more or less deliberately for the current design, but I am open for improvements:

A few arguments from my point of view:

I would also be interested where exactly you see the complexity in the particular feature.

@MagdaN
Copy link
Contributor Author

MagdaN commented Dec 15, 2016

I think the most complicated part is the URL design as it is not visible in the router that you are using a slug, but then the slug is magically added. I can understand that you want to avoid query parameters but as this url is not visible to users I think query parameters are ok.

For further development we should decide which way to go, because, I think, it is also confusing to use different approaches inside one API. If we decide to stick to this we should consider to refactor the other Apps (e.g. reports)

@SabineJulia
Copy link

related to #629

@SabineJulia SabineJulia added this to the v 2.5 release milestone May 18, 2017
@slomo slomo modified the milestones: v 2.5 release, v 2.6 release Jun 21, 2017
@philli-m philli-m removed this from the v 2.8 release milestone Jul 22, 2021
@hello-kathi
Copy link

@phillimorland Don't know what that is :) Please lable with reminder if it is important or close.

@philli-m
Copy link
Contributor

this is ref in follow issue but this is bit general for current level of development so will close

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

No branches or pull requests

5 participants