Skip to content
This repository was archived by the owner on Feb 27, 2025. It is now read-only.

Conversation

@alaney
Copy link

@alaney alaney commented Jan 19, 2024

Summary

This change allows multiple responses to be recorded for the same request.

For example, if an app makes a GET request to the endpoint /todo/1 it will record the response within a response collection for the calculated request ID.

Any subsequent responses for the same GET requests will be pushed into the response collection for that calculated request ID.

This allows the plugin to support the case described in the issue. Where a later request for an entity results in a different response.

Related Issue(s)

Issue #128

Checklist

This allows multiple responses to be recorded for the same request
@alaney alaney changed the title Introduce response collections Allow multiple responses for a unique request ID Jan 19, 2024
@alaney alaney changed the title Allow multiple responses for a unique request ID feat: Allow multiple responses for a unique request ID Feb 1, 2024
@alaney alaney changed the title feat: Allow multiple responses for a unique request ID feat!: Allow multiple responses for a unique request ID Feb 1, 2024
@alaney alaney marked this pull request as ready for review February 1, 2024 21:18
@alaney alaney requested a review from a team as a code owner February 1, 2024 21:18
@alaney
Copy link
Author

alaney commented Feb 8, 2024

@tvsbrent Pinging you on this to see if you have any feedback.

Copy link
Contributor

@tvsbrent tvsbrent left a comment

Choose a reason for hiding this comment

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

Thanks so much for this submission! We did have one suggestion that should make it easier to introduce this change.

Comment on lines 193 to 195
data.responseCollections.forEach(entry => {
const responseCollection = new PlaybackResponseCollection(entry);
this.#responseCollections.set(responseCollection.id, responseCollection);
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: We were discussing an approach that would keep us from having to introduce this as a breaking change. It's honestly something I should have thought of before, but here in the deserialize method for the PlaybackRequestMatcher, we could put in code to migrate the responses field into the responseCollections field.

I don't think such a migration would be that difficult. (Although, I've been wrong more than once!) Some pseudo-code below:

if (data.responses) {
  data.responses.forEach(entry => {
    const { id, url, ...rest } = entry;
    // This should match the shape that the response collection constructor/deserialize method expects
    const responseCollection = new PlaybackResponseCollection({ id, url, responses: [rest] });
    this.#responseCollection.set(responseCollection.id, responseCollection);
  });
} else {
  // Other deserialization code here.
  ...
}

https://github.com/oreillymedia/cypress-playback/blob/main/lib/commands/entities/PlaybackRequestMatcher.js#L198-L201

@alaney alaney changed the title feat!: Allow multiple responses for a unique request ID feat: Allow multiple responses for a unique request ID Feb 13, 2024
@alaney
Copy link
Author

alaney commented Feb 13, 2024

Your suggestion seems to work. I implemented it and added a few tests.

@tvsbrent tvsbrent merged commit 46dc7e1 into oreillymedia:main Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants