change PaidCourseRegistration.contained_in_order

[jbau]

now handles cases where the order has both a PaidCourseRegistration
and a CertificateItem

@chrisndodge @cdodge

[jbau]

I decided to use isinstance after all, because this is a function within the PaidCourseRegistration class

[chrisndodge]

Great! Thanks, looks like what I expected and thanks for the succeed/failed verification on the unit test (failed on old code, passed on new)

[jbau]

Test failures seems unrelated. They're in the JS unit tests and discussions:

17:18:04 DiscussionThreadView response count and pagination: correctly render for a thread with one response [fail]
17:18:04     Expected '' to equal '1 response'.
17:18:04     Expected '' to equal 'Showing all responses'.
17:18:04 
17:18:04 DiscussionThreadView response count and pagination: correctly render for a thread with one additional page [fail]
17:18:04     Expected '' to equal '2 responses'.
17:18:04     Expected '' to equal 'Showing first response'.
17:18:04     Expected '' to equal 'Load all responses'.
17:18:04 
17:18:04 DiscussionThreadView response count and pagination: correctly render for a thread with multiple additional pages [fail]
17:18:04     Expected '' to equal '111 responses'.
17:18:04     Expected '' to equal 'Showing first 2 responses'.
17:18:04     Expected '' to equal 'Load next 100 responses'.
17:18:04 
17:18:04 DiscussionThreadView response count and pagination on clicking the load more button: correctly re-render when all threads have loaded [fail]
17:18:04     Expected '' to equal '5 responses'.
17:18:04     Expected '' to equal 'Showing first response'.
17:18:04     Expected '' to equal 'Load all responses'.
17:18:04     Expected '' to equal '5 responses'.
17:18:04     Expected '' to equal 'Showing all responses'.
17:18:04 
17:18:04 DiscussionThreadView response count and pagination on clicking the load more button: correctly re-render when one page remains [fail]
17:18:04     Expected '' to equal '5 responses'.
17:18:04     Expected '' to equal 'Showing first response'.
17:18:04     Expected '' to equal 'Load all responses'.
17:18:04     Expected '' to equal '42 responses'.
17:18:04     Expected '' to equal 'Showing first 3 responses'.
17:18:04     Expected '' to equal 'Load all responses'.
17:18:04 
17:18:04 DiscussionThreadView response count and pagination on clicking the load more button: correctly re-render when multiple pages remain [fail]
17:18:04     Expected '' to equal '5 responses'.
17:18:04     Expected '' to equal 'Showing first response'.
17:18:04     Expected '' to equal 'Load all responses'.
17:18:04     Expected '' to equal '111 responses'.
17:18:04     Expected '' to equal 'Showing first 3 responses'.
17:18:04     Expected '' to equal 'Load next 100 responses'.
17:18:04 

I kicked off another manual test build

[chrisndodge]

Yea, I've seen those fail on other PR branches as well.

[jbau]

same tests failed again, blargh

[wedaly]

@jbau I ran into that issue the other day and was able to resolve it by rebasing onto master.

[jbau]

ok. i rebased on master

[chrisndodge]

@jzoldak @Lyla-Fischer FYI this is the fix we'd like to get on prod if there's going to be a hotfix going out this week. Should we merge this to master (when tests pass) and then potentially cherry-pick to the RC branch?

If there's no hotfix going out, then I wouldn't force one to happen to for this.

[jbau]

OK. Tests are finally passing

[chrisndodge]

+1