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

18053 Legal API - Validation (AGM Extension) #2298

Merged
merged 16 commits into from
Nov 15, 2023

Conversation

chenhongjing
Copy link
Collaborator

Issue #: /bcgov/entity#18053

Description of changes:

  • Implemented validation for AGM Extension
  • Added unit tests

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).

Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Merging #2298 (a1835a2) into main (79511cd) will decrease coverage by 0.34%.
Report is 22 commits behind head on main.
The diff coverage is 56.84%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2298      +/-   ##
==========================================
- Coverage   77.37%   77.03%   -0.34%     
==========================================
  Files         202      205       +3     
  Lines       11602    11931     +329     
  Branches     1961     2013      +52     
==========================================
+ Hits         8977     9191     +214     
- Misses       2043     2143     +100     
- Partials      582      597      +15     
Flag Coverage Δ
legalapi 76.46% <56.84%> (-0.38%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
legal-api/src/legal_api/config.py 95.16% <100.00%> (+0.28%) ⬆️
legal-api/src/legal_api/core/filing.py 86.55% <100.00%> (+0.04%) ⬆️
legal-api/src/legal_api/core/meta/filing.py 91.11% <ø> (ø)
legal-api/src/legal_api/models/__init__.py 100.00% <100.00%> (ø)
legal-api/src/legal_api/models/dc_definition.py 90.69% <100.00%> (ø)
legal-api/src/legal_api/services/authz.py 94.54% <ø> (ø)
...gal_api/services/filings/validations/alteration.py 83.01% <100.00%> (ø)
...al-api/src/legal_api/utils/legislation_datetime.py 84.12% <100.00%> (+0.79%) ⬆️
legal-api/src/legal_api/version.py 100.00% <100.00%> (ø)
..._services/entity-filer/src/entity_filer/version.py 100.00% <ø> (ø)
... and 10 more

... and 2 files with indirect coverage changes

Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
@chenhongjing chenhongjing marked this pull request as ready for review November 8, 2023 22:57
@chenhongjing chenhongjing self-assigned this Nov 8, 2023
Signed-off-by: Hongjing Chen <[email protected]>
Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

From an FE's perspective, I think it looks good! Hongjing, I'll let you know if anything comes up from my side.

from unittest.mock import patch

import pytest
from registry_schemas.example_data import AGM_EXTENSION, FILING_HEADER
Copy link
Collaborator

Choose a reason for hiding this comment

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

If AGM_EXTENSION isn't used you can probably remove it, otherwise looks good!

Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Signed-off-by: Hongjing Chen <[email protected]>
Comment on lines 168 to 170
if intended_agm_date > curr_ext_expire_date:
msg.append({'error': 'Intended AGM date should not be greater than current extension expiry date.',
'path': f'{AGM_EXTENSION_PATH}/intendedAgmDate'})
Copy link
Collaborator

Choose a reason for hiding this comment

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

so when i said intended_agm_date should not be greater than the extension expiry date, I probably wasn't clear enough. I meant the intended_agm_date should not be greater than whatever new expiry date you would be granting for the extension being requested.

Copy link
Collaborator

Choose a reason for hiding this comment

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

intended_agm_date validation can be removed as per Mihai's comment in the ticket.

Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
5.3% 5.3% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@chenhongjing chenhongjing merged commit d1fa5d7 into bcgov:main Nov 15, 2023
7 of 9 checks passed
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.

4 participants