Skip to content

Restrict methods#543

Open
Olegt0rr wants to merge 4 commits intomasterfrom
restrict-methods
Open

Restrict methods#543
Olegt0rr wants to merge 4 commits intomasterfrom
restrict-methods

Conversation

@Olegt0rr
Copy link
Copy Markdown
Collaborator

@Olegt0rr Olegt0rr commented Dec 2, 2024

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 2, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b6883cb) to head (d88dd03).
⚠️ Report is 68 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #543   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          475       487   +12     
  Branches        17        18    +1     
=========================================
+ Hits           475       487   +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Dreamsorcerer
Copy link
Copy Markdown
Member

I think that's overkill, we only want to ensure that the user understands that GET is the only allowed method for browsers. A boolean should suffice.

@Olegt0rr
Copy link
Copy Markdown
Collaborator Author

Olegt0rr commented Dec 3, 2024

Switched to bool

@Dreamsorcerer
Copy link
Copy Markdown
Member

I've added changelog as we don't seem to have any docs. I think that's fine, so feel free to merge if you're happy with it.

@Olegt0rr Olegt0rr marked this pull request as ready for review December 5, 2024 03:16
@Olegt0rr
Copy link
Copy Markdown
Collaborator Author

Olegt0rr commented Dec 5, 2024

@Dreamsorcerer, thanks

I got some thoughts :D

This way we helped to warn frontend developer with 405 status code after request.
But we still not warn backend developer. May be we need to also add a warning.warn?
Or fail not as answer to the front, but with RuntimeError?

@Dreamsorcerer
Copy link
Copy Markdown
Member

But we still not warn backend developer. May be we need to also add a warning.warn?

I would expect a backend developer to test their code, even if only with Postman or similar, which is why this probably still works.
But, if you have a suggestion to create an error for the developer earlier, then that'd be even better.

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

Successfully merging this pull request may close these issues.

2 participants