List membership enrollments always returns 404

[GeekJosh]

Reproduction Steps

Perform a GET request to the /memberships/{id}/enrollments endpoint using any valid membership post id

Expected Behavior

A collection of enrollments for the given membership is returned

Actual Behavior

The API returns 404 llms_rest_not_found

Error Messages / Logs

{
    "code": "llms_rest_not_found",
    "message": "The requested resource could not be found.",
    "data": {
        "status": 404
    }
}

System Information

System Report

LifterLMS Rest API 1.0.0-beta.12

[GeekJosh]

after realising that using the X-LLMS headers fixes the issue I reported in #179 I have just confirmed the same is true here. It seems that somewhere, Basic Auth header isn't being handled properly

[thomasplevy]

See my reply to your other issue at #179 (comment)

I think we have duplicate issues here

[thomasplevy]

I misread the title when reviewing the other issue and thought this was stating the the GET /memberships endpoint was the reported problem endpoint.

This actually seems to me to be completely unrelated to #179 (it's a 404 not a 401/3) and I am seeing issues here. It is 404ing regardless of whether or not basic auth or header auth is used.

Could you test you integration again and let me know what kind of result you're getting when using header auth.

You should be getting a 404 either way

[GeekJosh]

Using header Auth, I get the collection of enrollments as expected

[thomasplevy]

@GeekJosh hmmm...

I'm seeing a different issue and what you're reporting is very strange in this case.

Could you post some example requests you're making so I can look at your integration a little closer please?

[thomasplevy]

@GeekJosh

Using header Auth, I get the collection of enrollments as expected

Is it possible you're testing against different memberships?

I found the following:

lifterlms-rest/includes/server/class-llms-rest-enrollments-controller.php

Lines 210 to 213 in d9ec770

	// Specs require 404 when no course enrollments are found.
	if ( ! is_wp_error( $response ) && empty( $response->data ) ) {
	return llms_rest_not_found_error();
	}

Which in some capacity I know I must have read and reviewed but I can't find anything in the spec stating enrollment lists should respond with a 404 when no enrollments are found. So I think this is the bug we're having here.

This means that both basic auth and header auth should respond with a 200 for a membership with at least one enrolled student and should respond with a 404 for a membership with no enrollments.

Could you confirm if this is the case?

@eri-trabiccolo could you use your infallible memory to dredge up the conversation we had in which I told you to make this respond with a 404 (and then never documented as much) and why? Or is this a bug?

Looking at this today it feels like it should respond with an empty list (not a 404) but I don't recall the context of why we decided to make it respond with 404.

If it is a bug we should fix and if there's a good reason we decided on this (which again, I've forgotten) we need to update the spec to make this explicitly clear.

[eri-trabiccolo]

@thomasplevy that might actually be a bug but I implemented that way on purpose because that's what the spec say:
https://developer.lifterlms.com/rest-api/#tag/Memberships/paths/~1memberships~1{id}~1enrollments/get

similar thing for the courses.

You already know the spec say that, but you have to imagine that I implemented most of the resources based on what the spec said.

[thomasplevy]

Can you clarify where you're seeing that the spec says to return a 404 when the membership has no enrollments?

The 404 I see documented I think would be expected when the membership doesn't exist -- not when no enrollments exist, right?

[eri-trabiccolo]

Mmm well no to me it meant that the enrollments did not exist, so it's my wrong understanding of the specs: BUG.
Maybe the spec could be more "complete" if they said what was the actual resource that couldn't be found.

I did the same with:

• membership enrollments
• course enrollments
• section content
• course content

they must be fixed then! Sorry!

[thomasplevy]

Aha!

[thomasplevy]

@GeekJosh in light of this bug that we've identified can you triple check that your initially reported experience and behavior is accurate?

Can you clarify where you're seeing that the spec says to return a 404 when the membership has no enrollments?

The 404 I see documented I think would be expected when the membership doesn't exist -- not when no enrollments exist, right?

I think that you have misreported the issue as an auth issue but in fact what we have is a bug in our code returning a 404 instead of a 200 with an empty array

[thomasplevy]

@GeekJosh can you check in with me on my last comment. We have found a bug (but it's unrelated to your initial report) and I'd like some clarification from you before closing this out.

[GeekJosh]

Sorry for the delay - I can confirm that regardless of using header/basic auth, I get a 404 on an empty collection

[thomasplevy]

@GeekJosh thanks for confirming