Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added test package and refactored logic for eligibility checker #516

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

Dragos0000
Copy link
Collaborator

No description provided.

constraint_max_xsd_version = constraints[MAX_XSD_VERSION_KEY][0]
if notice_metadata.is_eform:
notice_xsd_version = notice_metadata.eform_sdk_version
eforms_sdk_version = notice_xsd_version.rsplit('-', 1)[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

this split need to be explained, maybe we need to use a template or regex selector instead of unclear split.

eform_subtype_constraint_values = [str(eforms_subtype_value) for eforms_subtype_value in
constraints[E_FORMS_SUBTYPE_KEY]]

in_date_range = constraint_start_date <= notice_publication_date <= constraint_end_date
in_version_range = constraint_min_xsd_version <= notice_xsd_version <= constraint_max_xsd_version
covered_eform_type = eform_subtype in eform_subtype_constraint_values

return True if in_date_range and in_version_range and covered_eform_type else False
Copy link
Collaborator

Choose a reason for hiding this comment

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

replace:
return True if in_date_range and in_version_range and covered_eform_type else False
with:
return in_date_range and in_version_range and covered_eform_type

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to remove reports from test package

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to remove reports from test package

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to remove reports from test package

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to remove reports from test package

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to remove reports from test package

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to remove reports from test package

Copy link
Collaborator

@CaptainOfHacks CaptainOfHacks left a comment

Choose a reason for hiding this comment

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

Review comments

@@ -79,9 +79,10 @@ class NormalisedMetadata(Metadata):
legal_basis_directive: str
form_number: str
eforms_subtype: str
xsd_version: str
xsd_version: Optional[str]
published_in_cellar_counter: int = Field(default=0)
is_eform: Optional[bool] = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to change this flag to a field:
notice_source: Optional[NoticeSource] = NoticeSource.STANDART_FORMS
and for NoticeSource as example:
`class NoticeSource(str, Enum):
STANDART_FORMS = "standart_forms"
ELECTRONIC_FORMS = "eforms"

def __str__(self):
    return self.value

`

@Dragos0000 Dragos0000 merged commit b03915a into main Jan 26, 2024
3 checks passed
@CaptainOfHacks CaptainOfHacks deleted the feature/TED4-85 branch January 27, 2024 12:53
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