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

Fix NamespaceVersioning ignoring DEFAULT_VERSION on non-None namespaces #7278

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

Kostia-K
Copy link
Contributor

@Kostia-K Kostia-K commented Apr 18, 2020

  • Fix the case where if the namespace is not None and there's no match,
    NamespaceVersioning always raises NotFound even if DEFAULT_VERSION
    is set or None is in ALLOWED_VERSIONS

  • Add test cases

Description

If my namespace is just 'api' (i.e. doesn't include version info), ALLOWED_VERSIONS is set to ['v1', 'v2'] and DEFAULT_VERSION is set to 'v2', I would expect request.version to resolve to 'v2'. The current behavior in this scenario is 404 Not Found which is inconsistent with the documentation.

Similarly, if DEFAULT_VERSION is not set and ALLOWED_VERSIONS includes None, I would expect request.version to be None. The current behavior is, once again, 404.

@stale
Copy link

stale bot commented Jun 19, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 19, 2022
@stale stale bot closed this Dec 26, 2022
@auvipy
Copy link
Member

auvipy commented Dec 26, 2022

If this is just going to fix bug without breaking any expected behavior then we might consider this

@auvipy auvipy added Bug and removed stale labels Dec 26, 2022
@auvipy auvipy reopened this Dec 26, 2022
@stale
Copy link

stale bot commented Apr 2, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 2, 2023
@auvipy auvipy removed the stale label Apr 3, 2023
@auvipy auvipy mentioned this pull request Apr 3, 2023
@auvipy auvipy self-requested a review May 17, 2023 14:08
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

can you please rebase?

@Kostia-K Kostia-K force-pushed the fix-namespace-versioning branch from 8ab39ec to 6fabaa3 Compare June 14, 2023 02:45
* Fix the case where if the namespace is not None and there's no match,
  NamespaceVersioning always raises NotFound even if DEFAULT_VERSION
  is set or None is in ALLOWED_VERSIONS

* Add test cases
@Kostia-K Kostia-K force-pushed the fix-namespace-versioning branch from 6fabaa3 to 7050b4d Compare June 14, 2023 02:55
@Kostia-K Kostia-K requested a review from auvipy June 14, 2023 03:06
@auvipy auvipy added this to the 3.15 milestone Jun 14, 2023
@auvipy
Copy link
Member

auvipy commented Jun 14, 2023

the patch looks good to me. I will take the time to review it thoroughly. I would really appreciate if you can do a pull review of this PR #8923

@auvipy auvipy merged commit 71f87a5 into encode:master Jun 14, 2023
auvipy added a commit that referenced this pull request Jun 17, 2023
* fix OpenAPIRenderer for timedelta

* added test for rendering openapi with timedelta

* fix OpenAPIRenderer for timedelta

* added test for rendering openapi with timedelta

* Removed usage of field.choices that triggered full table load (#8950)

Removed the `{{ field.choices|yesno:",disabled" }}` block because this triggers the loading of full database table worth of objects just to determine whether the multi-select widget should be set as disabled or not.

Since this "disabled" marking feature is not present in the normal select field, then I propose to remove it also from the multi-select.

* Added Deprecation Warnings for CoreAPI (#7519)

* Added Deprecation Warnings for CoreAPI

* Bumped removal to DRF315

* Update rest_framework/__init__.py

* Update rest_framework/filters.py

* Update rest_framework/filters.py

* Update tests/schemas/test_coreapi.py

* Update rest_framework/filters.py

* Update rest_framework/filters.py

* Update tests/schemas/test_coreapi.py

* Update tests/schemas/test_coreapi.py

* Update setup.cfg

* Update rest_framework/pagination.py

---------

Co-authored-by: Asif Saif Uddin <[email protected]>

* Update copy right timeline

* Fix NamespaceVersioning ignoring DEFAULT_VERSION on non-None namespaces (#7278)

* Fix the case where if the namespace is not None and there's no match,
  NamespaceVersioning always raises NotFound even if DEFAULT_VERSION
  is set or None is in ALLOWED_VERSIONS

* Add test cases

* fix OpenAPIRenderer for timedelta

* added test for rendering openapi with timedelta

* added testcase for rendering yaml with minvalidator for duration field (timedelta)

---------

Co-authored-by: Rizwan Shaikh <[email protected]>
Co-authored-by: Lenno Nagel <[email protected]>
Co-authored-by: David Smith <[email protected]>
Co-authored-by: Asif Saif Uddin <[email protected]>
Co-authored-by: Konstantin Kuchkov <[email protected]>
@auvipy
Copy link
Member

auvipy commented Mar 22, 2024

we got a regression/breaking change report

@Kostia-K
Copy link
Contributor Author

we got a regression/breaking change report

Thanks for letting me know. Responded in the issue comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants