Skip to content

Conversation

@swibi-ttd
Copy link

@swibi-ttd swibi-ttd commented Feb 13, 2026

API Changes:

  • GET /api/partner_config/list - List all partner configs
  • GET /api/partner_config/get/{partner_name} - Get specific partner config
  • POST /api/partner_config/add - Add new partner
  • PUT /api/partner_config/update - Update existing partner
  • DELETE /api/partner_config/delete - Delete partner
  • POST /api/partner_config/bulk_replace - Replace all configs (renamed from update and moved to Danger Zone)

UI Changes:

  • Updated operations in OptOut Partner Management page to reflect API changes

private void handlePartnerConfigGet(RoutingContext rc) {
try {
final String partnerName = rc.pathParam("partner_name");
if (partnerName == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also check if this is an empty string

Copy link
Author

Choose a reason for hiding this comment

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

Added check for empty string

}

String config = this.partnerConfigProvider.getConfig();
JsonArray allPartnerConfigs = new JsonArray(config);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put these into one line like you do for the other methods

JsonArray allPartnerConfigs = new JsonArray(config);

// Look for the specific partner
for (int i = 0; i < allPartnerConfigs.size(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You reuse a similar logic in most of these methods, can we look at putting this into a reusable function

Copy link
Author

Choose a reason for hiding this comment

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

Added findPartnerIndex helper fn


private void handlePartnerConfigAdd(RoutingContext rc) {
try {

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Remove this space


rc.response()
.putHeader(HttpHeaders.CONTENT_TYPE, "application/json")
.end("\"success\"");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we respond with the new partner config that we added

Copy link
Author

Choose a reason for hiding this comment

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

Changed to respond with new config


rc.response()
.putHeader(HttpHeaders.CONTENT_TYPE, "application/json")
.end("\"success\"");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we respond with the new updated object

Copy link
Author

Choose a reason for hiding this comment

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

Changed to respond with updated object

ResponseUtil.error(rc, 400, "Partner name is required");
return;
}
final String partnerName = partnerNames.getFirst();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we accept a list of partnerNames but only remove the first partner?

Copy link
Author

Choose a reason for hiding this comment

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

I think RoutingContext.queryParam is a list of Strings since you can always specify multiple values for each query param eg. /delete/?partner_name=partner1&partner_name=partner2 despite only expecting one partner_name here. src/main/java/com/uid2/admin/vertx/service/SiteService.java has the same behaviour in handleSiteAdd

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks

private void handlePartnerConfigBulkReplace(RoutingContext rc) {
try {
// refresh manually
this.partnerConfigProvider.loadContent();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we refresh the provider manually for all methods before retrieving the config?

Copy link
Author

Choose a reason for hiding this comment

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

Added manual refresh in all methods

try {

JsonObject newConfig = rc.body().asJsonObject();
if (newConfig == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put this check in the validatePartnerConfig method as well?

Copy link
Author

@swibi-ttd swibi-ttd Feb 13, 2026

Choose a reason for hiding this comment

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

Yep added null check to validatePartnerConfig. I've left the existing checks in so we can give better errors

storageManager.upload(allPartnerConfigs);
rc.response()
.putHeader(HttpHeaders.CONTENT_TYPE, "application/json")
.end("\"success\"");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also return the deleted object here? We can return a new object similar to what we do for ClientSideKeyPairs like:
{
success: true,
deleted_partner: allPartnerConfigs.get(existingPartnerIdx)
}

Copy link
Author

Choose a reason for hiding this comment

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

Updated to return deleted object. Do you think it's sufficient to return 200 + deleted object or would you still prefer to add success: true as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that's fine, returning the deleted object was the main thing I wanted!


storageManager.upload(partners);

rc.response()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also return the replaced partner config here

Copy link
Author

Choose a reason for hiding this comment

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

Now returns replace config

}
}, new AuditParams(Collections.emptyList(), List.of("partner_id", "config")), Role.PRIVILEGED));
}, new AuditParams(Collections.emptyList(), List.of("name")), Role.MAINTAINER));
router.delete(API_PARTNER_CONFIG_DELETE.toString()).blockingHandler(auth.handle((ctx) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets also put the DELETE operation in the danger zone as a PRIVILEGED role

Copy link
Author

Choose a reason for hiding this comment

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

Moved to danger zone and updated to Role.PRIVILEGED

this.handlePartnerConfigDelete(ctx);
}
}, new AuditParams(List.of("partner_name"), Collections.emptyList()), Role.MAINTAINER));
router.post(API_PARTNER_CONFIG_BULK_REPLACE.toString()).blockingHandler(auth.handle((ctx) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make bulk replace for Role.SUPERUSER

Copy link
Author

Choose a reason for hiding this comment

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

Updated to Role.SUPERUSER

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