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

psp-7034 add warning when interest holders attached to acquisition pr… #3550

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

devinleighsmith
Copy link
Collaborator

…operty being deleted.

@devinleighsmith devinleighsmith added the bug Something isn't working label Oct 27, 2023
@devinleighsmith devinleighsmith self-assigned this Oct 27, 2023
@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #3550 (65abc4b) into dev (abb9278) will decrease coverage by 6.25%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3550      +/-   ##
==========================================
- Coverage   69.30%   63.05%   -6.25%     
==========================================
  Files        1370      471     -899     
  Lines       33538    15040   -18498     
  Branches     6229     1019    -5210     
==========================================
- Hits        23242     9483   -13759     
+ Misses      10046     5307    -4739     
  Partials      250      250              
Flag Coverage Δ
unittests 63.05% <100.00%> (-6.25%) ⬇️

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

Files Coverage Δ
...rce/backend/api/Services/AcquisitionFileService.cs 72.99% <100.00%> (ø)
.../Repositories/AcquisitionFilePropertyRepository.cs 97.95% <100.00%> (+0.04%) ⬆️

... and 900 files with indirect coverage changes

Comment on lines +299 to +301
if (acqFileProperties.PimsTakes.Any() || acqFileProperties.PimsInthldrPropInterests.Any())
{
throw new BusinessRuleViolationException();
throw new BusinessRuleViolationException("You must remove all takes and interest holders from an acquisition file property before removing that property from an acquisition file");
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: would love to see a unit test covering this bug fix

@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@devinleighsmith devinleighsmith merged commit 6ded456 into bcgov:dev Oct 30, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants