feat: add missing options parameters to CreateConnectTokenRequest#65
feat: add missing options parameters to CreateConnectTokenRequest#65cauecalil wants to merge 1 commit intopluggyai:masterfrom
Conversation
Added 'oauthRedirectUri' and 'avoidDuplicates' to the Options class to align the SDK with the current Pluggy API documentation. Existing constructors were preserved to guarantee backward compatibility and prevent breaking changes for current users.
Gabrielpanga
left a comment
There was a problem hiding this comment.
🤖 AI Review
The change looks correct for adding the two missing API parameters. A few observations:
Looks good:
- Using
Boolean(boxed) foravoidDuplicatesis the right call — it staysnullby default and will be omitted from serialized JSON, avoiding unintendedfalsevalues. - Lombok's
@AllArgsConstructor+@DataonOptionsmakes this clean. - Existing constructors in
CreateConnectTokenRequestare updated to passnull, null, preserving backward compatibility.
Suggestion — exposing the new fields:
Right now, there's no way for SDK users to actually set oauthRedirectUri or avoidDuplicates through CreateConnectTokenRequest. The existing constructors always pass null for both new fields, and there are no new constructors or setters that accept them. Consider adding a new constructor (or a builder) to CreateConnectTokenRequest that forwards these parameters, e.g.:
public CreateConnectTokenRequest(String itemId, String webhookUrl, String clientUserId, String oauthRedirectUri, Boolean avoidDuplicates) {
this.itemId = itemId;
this.options = new Options(webhookUrl, clientUserId, oauthRedirectUri, avoidDuplicates);
}Without this, the new fields are defined but effectively unreachable from the public API of the SDK.
Minor: If you want to keep the old 2-param Options constructor available for other callers, you could add a secondary constructor or use Lombok's @Builder instead of relying solely on @AllArgsConstructor. Not critical since Options appears to be internal.
Overall this is a solid, focused change. The main gap is the missing constructor/builder in CreateConnectTokenRequest to expose the new parameters to SDK consumers.
Gabrielpanga
left a comment
There was a problem hiding this comment.
🤖 AI Review — Update
Changing this to request changes.
The new oauthRedirectUri and avoidDuplicates fields are added to the Options class, but there's no way for SDK users to actually set them — the CreateConnectTokenRequest constructors always pass null for both parameters.
As an open-source SDK, we'd love contributions that deliver end-to-end value rather than partial patches. Could you please also add a constructor or builder method that exposes these new fields to consumers? That way users can actually benefit from this change without needing a follow-up PR.
Without that, the fields are effectively dead code from the SDK user's perspective.
Thanks for the detailed review! The new fields are already reachable by SDK consumers via the Options options = Options.builder()
.clientUserId(clientUserId)
.avoidDuplicates(true)
.build();
CreateConnectTokenRequest request = CreateConnectTokenRequest.builder()
.itemId(itemId)
.options(options)
.build();The legacy constructors were intentionally kept unchanged to avoid breaking existing consumers who instantiate the class directly, the new fields simply default to So no additional constructor should be needed here. Let me know if you see something I'm missing! |
I was using the SDK and noticed that the
Optionsclass for creating connect tokens was missing a couple of parameters compared to the current API docs.This PR adds
oauthRedirectUriandavoidDuplicatesto theOptionspayload.What changed:
Options.java.CreateConnectTokenRequest.java.Note on compatibility: I kept the original legacy constructors in
CreateConnectTokenRequestexactly as they were so this doesn't introduce any breaking changes for existing users. The new fields just default to null if the old constructors are used.Let me know if you need me to adjust anything!