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 api resource names #29

Merged
merged 1 commit into from
Jan 6, 2020
Merged

Fix api resource names #29

merged 1 commit into from
Jan 6, 2020

Conversation

allisson
Copy link
Owner

@allisson allisson commented Jan 6, 2020

No description provided.

@allisson allisson merged commit 29ca064 into master Jan 6, 2020
@allisson allisson deleted the v1.0.3 branch January 6, 2020 21:27
Copy link
Contributor

@leorochael leorochael left a comment

Choose a reason for hiding this comment

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

The idea behind this PR is excellent, but the changes kind-of threw the baby out with the bathwater.

Previously, resources with dashes in the name were impossible because they generated invalid attribute names.

Now they're impossible because the "valid" resource name with underscores ends up in the URL, instead of the original resource name with dashes, so using the resource as is causes 404 exceptions, as was reported later in #32.

resource_class = resource_class or Resource
resource = resource_class(
api_root_url=api_root_url or self.api_root_url,
resource_name=resource_name,
resource_name=resource_valid_name,
Copy link
Contributor

Choose a reason for hiding this comment

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

This line change should be reverted, so the resource itself retains the original name.
Otherwise accessing the resource results in 404 errors:

Suggested change
resource_name=resource_valid_name,
resource_name=resource_name,

valid_name = self.correct_attribute_name(resource_name)
self._resources[valid_name] = resource
setattr(self, valid_name, resource)
self._resources[resource_valid_name] = resource
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that the _resources dictionary should have the original resource_name as key, rather than the fixed resource name.

Suggested change
self._resources[resource_valid_name] = resource
self._resources[resource_name] = resource

(though actually this line change happened in a previous commit: #28).

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.

None yet

2 participants