Make /communication_cost/ endpoint with OffsetPageSchema

[jason-upchurch]

Updated summary

This work makes the new endpoint /communication_costs/ available. Subsequent removal of the old /communication-costs/ endpoint will happen under another issue, and is marked as a completion criteria for this issue.

Original Summary

This ticket continues the work previously done to bring the communications-cost endpoint in line with the typical pagination scheme used throughout our API, i.e., the OffsetPageSchema, as opposed to the schema used for endpoints for larger datasets, i.e., the SeekPageSchema.

The main work remaining to be done under this issue is the "appropriate" handling of making the change to the endpoint which represents a breaking change. The current work is preserved under WIP PR #4232

Background from previous work

api endpoints that support pagination through page, some. endpoints make the page key explicitly available (e.g., /communication_costs/totals/by_candidate/) while others do not (e.g., /communication-costs/).

For uniformity and to support generalizing programmatic access to our API, we should make pagination keys when possible (a case where this would not be currently possible, for example, would be the schedule_a endpoints where additional keys are needed to support pagination.

Action items

• add a new /communication_costs/ endpoint with OffsetPageSchema
• ensure any bug fixes on original /communication-costs/ are in place at least in this work, e.g., Fix sort on candidate and committee name #4529 cc @lbeaufort @patphongs existing bugs are with /communication_costs/aggregates/ which uses aggregates.CCAggregatesView ( /communication-costs/ uses costs.CommunicationCostView)
• locate all the places where calls exist to /communication-costs/ (e.g., endpoint walk) and open issues to repoint to new endpoint
• make cms issue to make correct API calls (Replace calls to /communication-costs/ with /communication_costs/ fec-cms#3963)
• send updated email to api users letting them know new endpoint is available
• update swagger docs for deprecation notice to add info about new endpoint (this is "partly" done--we added a note about a future endpoint but have not put the endpoint in place--this criteria is to add language to indicate that they should begin using the new endpoint)
• open issue to remove old endpoint /communication-costs/ and make a completion criteria of letting api users know and to not actually remove before the timeframe indicated in our deprecation notice.
• open issue to have team discussion about deprecation policy when we need to make a fundamental change to an endpoint (e.g., pagination schema) and are not willing/able to easily change the name of the endpoint (having the name of this endpoint be "out of sync" with other communication_costs endpoints was helpful, in a way)

Completion criteria:

• New endpoint is created with offsetpageschema

[jason-upchurch]

There are 4 endpoints under the communication cost tag:

/committee/{committee_id}/communication_costs/by_candidate/, /communication-costs/, /communication_costs/by_candidate/, ​/communication_costs​/totals​/by_candidate​/. Only /communication-costs/ does not have an explicit page key in pagination. Will investigate what is different about the paging for this endpoint and any rationale.

[jason-upchurch]

The /communication-costs/ endpoint currently uses the SeekPageSchema. These are the other schemas using SeekPageSchema (from marshmallow-pagination):

• ./schemas.py:ScheduleAPageSchema = make_page_schema(ScheduleASchema, page_type=paging_schemas.SeekPageSchema)
• ./schemas.py:ScheduleBPageSchema = make_page_schema(ScheduleBSchema, page_type=paging_schemas.SeekPageSchema)
• ./schemas.py:ScheduleH4PageSchema = make_page_schema(ScheduleH4Schema, page_type=paging_schemas.SeekPageSchema)
• ./schemas.py:ScheduleEPageSchema = make_page_schema(ScheduleESchema, page_type=paging_schemas.SeekPageSchema)
• ./schemas.py:ElectioneeringPageSchema = make_page_schema(ElectioneeringSchema, page_type=paging_schemas.SeekPageSchema)

[jason-upchurch]

@pkfec @fec-jli @lbeaufort @hcaofec cc @PaulClark2, The issue here is that a user reported inconsistent pagination between communications cost endpoints (there are 4). Three endpoints use the OffsetPageSchema from marshmallow-pagination while the base endpoint /communication-costs/ uses the SeekPageSchema. The difference is that the OffsetPage uses page number for pagination, while the SeekPage uses the keyset pagination found in Schedule A, for example.

When no argument is supplied to make_page_schema the default is OffsetPageSchema. The majority of our endpoints use this pagination schema. I found that the SeekPageSchema appears to be used in conjunction with our large dataset tables, e.g., Schedule A. These page schemas were implemented prior to my supporting the API, so I'm copying you in case there is any institutional memory for the motivation for choosing the particular paging schema (SeekPageSchema) for the /communication-cost/ endpoint while using the more common OffsetPageSchema for the others.

If there is a specific reason we can probably close this issue and note it in case it comes up again. If there is not, I'd suggest we bring it in line with the other endpoints and notify API users of a potential breaking change in the pagination. If this is not acceptable, perhaps we can create another endpoint that duplicates /communication-cost/ with the new endpoint using the more common paging schema.

[lbeaufort]

Here's our deprecation policy, which is definitely open for improvement: https://github.com/fecgov/openFEC/wiki/Deprecation-policy

[jason-upchurch]

Thanks @lbeaufort, Thanks for linking the doc! The main (not insurmountable) obstacle to following this deprecation policy is that we aren't replacing and endpoint. Still, good info and we can certainly let api users know far in advance.

[lbeaufort]

We've started using the deprecation policy for breaking changes, since removing an endpoint and making a breaking change both have detrimental impacts on the user. They're definitely guidelines though and not a strict policy.

[jason-upchurch]

We've started using the deprecation policy for breaking changes, since removing an endpoint and making a breaking change both have detrimental impacts on the user. They're definitely guidelines though and not a strict policy.

@lbeaufort what portions of the guidance would you specifically recommend for this issue beyond notifying the api users? We can incorporate that into the issue. Thanks!

[lbeaufort]

Personally, I'd follow the policy and optionally adjust the timing based on my best judgement.

[jason-upchurch]

Personally, I'd follow the policy and optionally adjust the timing based on my best judgement.

Thanks @lbeaufort if you happen to review the doc and find any actionable recommendations we can consider them. Otherwise I'll proceed with letting api users know in advance and target a timeframe. Thanks for your help trying to apply the deprecation to the pagination issue.