Breaking: Fix publishing multiple rev reg defs with endorsement

[jamshale]

This fixes a bug/mistake when publishing revocations from multiple rev_reg_defs with endorsement. The endorsement process was occurring outside the manager and only considering the last rev_reg_def pre-endorsement result.

Note: This would be considered a breaking change if a controller was reading and processing the response. It is now an array instead of an object. This should have been the response from the beginning as you can request a list, you should return a list. Could be a nested object instead if that's preferable.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[jamshale]

@swcurran I left a note about the change in the response payload. Should be in the next release docs. Not sure if any controllers would do anything with the response, but they might.

[swcurran]

Sounds good. I’ll put it into the breaking change notes for the release.

[cl0ete]

@jamshale
Given the response model for publish_revocations:

class TxnOrPublishRevocationsResultSchema(OpenAPISchema):
    """Result schema for credential definition send request."""

    sent = fields.Nested(
        PublishRevocationsSchema(),
        required=False,
        metadata={"definition": "Content sent"},
    )
    txn = fields.Nested(
        TransactionRecordSchema(),
        required=False,
        metadata={
            "description": "Revocation registry revocations transaction to endorse"
        },
    )

Should the model not be updated to handle the fact that txn can now be a list.
And should "rrid2crid" not be txn on line 669 of aries_cloudagent/revocation/routes.py

[jamshale]

I guess I forgot to update the response model here. The response is different depending on whether or not an endorsed transaction was used. If not using endorsement then it had the rrid2crid response. I didn't change this. This endpoint could still be better and have a more consistent response values, but it should work as expected.

I agree the response model should at least be defined as a list. Feel free to open a PR for it or I will get to it shortly.

[ff137]

@jamshale we'll make a PR for it 👌
Busy testing a fix to the response model with our "cloudcontroller" client, generated from the openapi spec.
(Today's a public holiday here, so will be next week)