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.

Sign up for GitHub

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

Jump to bottom

fix(model): throw an error if operations are used with custom #173

Merged
merged 5 commits into from
Jan 19, 2021
Merged

fix(model): throw an error if operations are used with custom #173

merged 5 commits into from
Jan 19, 2021

Conversation

JoaoPedroAS51
Copy link
Collaborator

@JoaoPedroAS51 JoaoPedroAS51 commented Jan 19, 2021 •
edited
Loading

Throw an error if CRUD or relationship operations are used in in conjunction with the custom method.

Fixes #172

robsontenorio
robsontenorio approved these changes Jan 19, 2021
View reviewed changes
Copy link
Owner

@robsontenorio robsontenorio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JoaoPedroAS51 !

@Peter-Krebs
Copy link
Collaborator

I don't think it has been reported before, but should we also implement this for delete() as well?
Just to prevent future issues with the same cause.

@JoaoPedroAS51
Copy link
Collaborator Author

JoaoPedroAS51 commented Jan 19, 2021 •
edited
Loading

@Peter-Krebs Good idea! Perhaps we should also implement this for attach and sync methods.

I noted that all of these methods (save, delete, attach and sync), are using this.endpoint() to set the url. So that's why I think we should implement this for all of them.

What do you think?

@JoaoPedroAS51 JoaoPedroAS51 changed the title fix(model): throw an error if save is used with custom fix(model): throw an error if operations are used with custom Jan 19, 2021
@Peter-Krebs
Copy link
Collaborator

Yes, anything that uses POST, PATCH, PUT or DELETE makes sense to me.

The error message should also say "Use for() instead." at the end so people know what to do.
Otherwise they will open up tickets again every time.

@JoaoPedroAS51
Copy link
Collaborator Author

@Peter-Krebs Done, error message updated :)

@Peter-Krebs
Copy link
Collaborator

Seems good, thanks for the fix!

@JoaoPedroAS51 JoaoPedroAS51 merged commit eef91b1 into dev Jan 19, 2021
@JoaoPedroAS51 JoaoPedroAS51 deleted the fix/save-custom-error branch January 19, 2021 13:39
@github-actions
Copy link

🎉 This PR is included in version 1.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Peter-Krebs Peter-Krebs Peter-Krebs approved these changes

@robsontenorio robsontenorio Awaiting requested review from robsontenorio

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Calling custom() before save() should trigger a console error
3 participants
@JoaoPedroAS51 @Peter-Krebs @robsontenorio