get_folder receives 404 when passing the name of the folder

[ederlf]

Trying to query a folder by the name will fail with 404.

folder = guru_client.get_folder("Folder", "Collection")

After a bit of debugging, this is the URL we look for.
'https://api.getguru.com/api/v1/folders/Folder.

If I try to use that url path, directly curling the API, I get a 404 too.
To have that working with the API, I had to use the id of the folder that shows up on Guru's URL of the folder:
``'https://api.getguru.com/api/v1/folders/`

The docs say get_folder can take either the name or the id, but it does not seem the case.

[markhornak]

Hi @ederlf - just tested again internally here and get_folder("{your folder name here}") works as expected!

is the code above the actual code you are running? That is saying find a folder named "Folder" in a collection called "Collection". Is that correct?

If "Collection" doesn't exist as you should get a message stating so.

[dephekt]

It looks like the issue is a lack of URL quoting when using a slug rather than a folder UUID:

    folder = g.get_folder("ckdGkL5i/About-the-Sales-team")

This returns a 404.

    slug = urllib.parse.quote("ckdGkL5i/About-the-Sales-team", safe="")
    folder = g.get_folder(slug)

This works.

It might be worth it to always call quote() on the incoming folder ID before passing it to the __get method. It shouldn't cause any harm when it isn't needed, but will allow this to work without the caller having to know this nuance.

[dephekt]

Actually, I can see something else weird is going on. When we preprocess the URL by quoting it properly, this causes it to fail the is_id regex checks that try to identify it as a slug or UUID. Because of that, it resorts to searching all the folders looking for a folder name (using our quoted slug, which fails because the function doesn't expect to receive a slug). And ultimately fails entirely to find a match.

So it seems like passing a slug is impossible, because if we quote it before calling get_folder, it will fail to find the folder by name and if we don't quote it, the SDK never does either and passes it directly in to the API request unquoted, which 404s.

The REST API requires that a slug with a backslash is properly quoted when you are calling /folders/{folderId}, but the SDK isn't ensuring this happens.

[dephekt]

You can fix this by making this change:

url = "%s/folders/%s" % (self.base_url, quote(folder_id, safe=""))

Right here:

py-sdk/guru/core.py

Line 2259 in 6c864e1

url = "%s/folders/%s" % (self.base_url, folder_id)

At which point you'll always url-quote the folder name, which is probably safe.

If you only want to url-quote when you have a slug, then you need to refactor this code to utilize the underlying is_slug and is_uuid checks, so you still have that information in this frame of reference and can then quote the folder ID accordingly, e.g.:

if is_slug(folder):
    folder_id = quote(folder, safe="")
elif is_uuid(folder):
    folder_id = folder
else:
    ...

Also, as an aside, this whole function could be refactored:

py-sdk/guru/core.py

Lines 97 to 104 in 6c864e1

	def is_id(value):
	if is_slug(value):
	return True
	else:
	if is_uuid(value):
	return True
	else:
	return False

def is_id(value):
    if is_slug(value):
        return True
    if is_uuid(value):
        return True
    return False

The elses here were unnecessary logic branches; if the condition were true, we'd return and never evaluate the remaining conditions, so in general having elses after returns like this is illogical.