Remove open licence references #2345
Conversation
| router_v0.register( | ||
| "ogl", | ||
| licence_views.OpenGeneralLicenceListDW, | ||
| basename="dw-ogl-only", | ||
| ) |
There was a problem hiding this comment.
this is likely to break pipelines, best to delete the pipeline first.
There was a problem hiding this comment.
I don't see any v0/ pipelines in the data-flow repo, only v1/. Are they defined somewhere else?
There was a problem hiding this comment.
ok then we are good. All of the pipelines we added are here https://github.com/uktrade/data-flow/blob/main/dags/data_infrastructure/lite_pipeline.py
hnryjmes
force-pushed
the
remove_open_licence_references_20241213
branch
from
f54ca26 to
666d91d
Compare
hnryjmes
force-pushed
the
remove_open_licence_references_20241213
branch
from
666d91d to
b8412d6
Compare
| @@ -82,7 +75,6 @@ class HMRCIntegrationLicenceSerializer(serializers.Serializer): | |||
| end_date = serializers.DateField() | |||
| organisation = HMRCIntegrationOrganisationSerializer(source="case.organisation") | |||
| end_user = HMRCIntegrationEndUserSerializer(source="case.baseapplication.end_user.party") | |||
| countries = serializers.SerializerMethodField() | |||
There was a problem hiding this comment.
we only needed to send countries for open licences. this field was being removed for standard licences (https://github.com/uktrade/lite-api/blob/dev/api/licences/serializers/hmrc_integration.py#L102) so removing this field from the serializer has no effect as we only currently support standard licences.
There was a problem hiding this comment.
I suggest to keep this field as is as removing it can break the receiving end on lite-hmrc
There was a problem hiding this comment.
I thought something like this this initially but you can see from looking at any LicencePayload data that we don't send countries ever because we only send SIELs. The reason being the code I linked above, i.e.
if (
hasattr(self.instance.case, "baseapplication")
and not (
hasattr(self.instance.case.baseapplication, "openapplication")
and self.instance.case.baseapplication.openapplication.application_countries.exists()
)
and not hasattr(self.instance.case, "opengenerallicencecase")
):
self.fields.pop("countries")
So keeping countries even if you set it via get_countries to some dummy value like [], would not have any effect, because with the existing code it gets removed anyway. I'm just making that explicit by removing it from the serializer.
The code I deleted for get_countries is only written for the scenario that an open licence is being sent:
def get_countries(self, instance):
if hasattr(instance.case, "baseapplication") and hasattr(instance.case.baseapplication, "openapplication"):
countries = get_approved_countries(instance.case.baseapplication)
else:
countries = instance.case.opengenerallicencecase.open_general_licence.countries.order_by("name")
return HMRCIntegrationCountrySerializer(
countries,
many=True,
).data
There is no version you could write of this for standard licences, it is just never run for those. So best to remove it I think.
There was a problem hiding this comment.
ok. I was thinking about temp siels where this may be useful. Is this really need to be removed for this PR?
Aim
Before we begin work on adding support for open licences (e.g. OIELs) to LITE it is useful to remove the old legacy code written for open licences which is not used.
There are other bits of unused code that could possibly be removed but this PR is already quite large so have tried to keep it just to open licence code for now.