Skip to content

feat: add missing options parameters to CreateConnectTokenRequest#65

Open
cauecalil wants to merge 1 commit intopluggyai:masterfrom
cauecalil:feat/add-create-connect-token-options
Open

feat: add missing options parameters to CreateConnectTokenRequest#65
cauecalil wants to merge 1 commit intopluggyai:masterfrom
cauecalil:feat/add-create-connect-token-options

Conversation

@cauecalil
Copy link
Copy Markdown

I was using the SDK and noticed that the Options class for creating connect tokens was missing a couple of parameters compared to the current API docs.

This PR adds oauthRedirectUri and avoidDuplicates to the Options payload.

What changed:

  • Added the two missing fields to Options.java.
  • Updated the internal call in CreateConnectTokenRequest.java.

Note on compatibility: I kept the original legacy constructors in CreateConnectTokenRequest exactly 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!

  • Code compiles correctly.
  • Tests run successfully locally.

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.
Copy link
Copy Markdown
Member

@Gabrielpanga Gabrielpanga left a comment

Choose a reason for hiding this comment

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

🤖 AI Review

The change looks correct for adding the two missing API parameters. A few observations:

Looks good:

  • Using Boolean (boxed) for avoidDuplicates is the right call — it stays null by default and will be omitted from serialized JSON, avoiding unintended false values.
  • Lombok's @AllArgsConstructor + @Data on Options makes this clean.
  • Existing constructors in CreateConnectTokenRequest are updated to pass null, 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.

Copy link
Copy Markdown
Member

@Gabrielpanga Gabrielpanga left a comment

Choose a reason for hiding this comment

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

🤖 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.

@cauecalil
Copy link
Copy Markdown
Author

🤖 AI Review

The change looks correct for adding the two missing API parameters. A few observations:

Looks good:

  • Using Boolean (boxed) for avoidDuplicates is the right call — it stays null by default and will be omitted from serialized JSON, avoiding unintended false values.
  • Lombok's @AllArgsConstructor + @Data on Options makes this clean.
  • Existing constructors in CreateConnectTokenRequest are updated to pass null, 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.

Thanks for the detailed review!

The new fields are already reachable by SDK consumers via the @Builder that exists on both CreateConnectTokenRequest and Options. Users who need oauthRedirectUri or avoidDuplicates can use the builder pattern directly:

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 null in those cases, which means they'll be omitted from the serialized JSON (as the review itself acknowledged for the Boolean type).

So no additional constructor should be needed here. Let me know if you see something I'm missing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants