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-8414 : Select one or multiple properties for H120 Generation #4160

Merged
merged 14 commits into from
Jul 10, 2024

Conversation

eddherrera
Copy link
Collaborator

@eddherrera eddherrera commented Jul 5, 2024

image

image

@eddherrera eddherrera added the enhancement New feature or request label Jul 5, 2024
@eddherrera eddherrera self-assigned this Jul 5, 2024
Copy link
Contributor

github-actions bot commented Jul 5, 2024

✅ No secrets were detected in the code.

@eddherrera
Copy link
Collaborator Author

eddherrera commented Jul 5, 2024

*** New template added to confluence page. ***

Copy link
Contributor

github-actions bot commented Jul 5, 2024

✅ No secrets were detected in the code.

Copy link
Contributor

github-actions bot commented Jul 5, 2024

✅ No secrets were detected in the code.

Herrera added 2 commits July 9, 2024 09:32
# Conflicts:
#	source/frontend/src/constants/API.ts
#	source/frontend/src/features/mapSideBar/acquisition/tabs/compensation/update/__snapshots__/UpdateCompensationRequisitionForm.test.tsx.snap
Copy link
Contributor

github-actions bot commented Jul 9, 2024

✅ No secrets were detected in the code.

@devinleighsmith
Copy link
Collaborator

Trying to remove a property that is on a compensation throws an unhandled db exception. Probably want to add to this logic:

if (acqFileProperties.PimsTakes.Any() || acqFileProperties.PimsInthldrPropInterests.Any()) { throw new BusinessRuleViolationException("You must remove all takes and interest holders from an acquisition file property before removing that property from an acquisition file"); }

@devinleighsmith
Copy link
Collaborator

The implemented view display of selected properties doesn't match figma and will probably be brought up as a bug by the FT team. Is this something you discussed with Ana, and is that documented on the story somewhere?
implementation:
image
mockup:
image

@devinleighsmith
Copy link
Collaborator

Also this AC is not met:
image

@devinleighsmith
Copy link
Collaborator

@eddherrera if I select/deselect multiple properties at a time the current implementation only seems to log one of my selections/deselections when I save.

Copy link
Collaborator

@devinleighsmith devinleighsmith left a comment

Choose a reason for hiding this comment

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

some mismatched requirements, functional issues.

Copy link
Contributor

✅ No secrets were detected in the code.

Copy link
Contributor

✅ No secrets were detected in the code.

@eddherrera
Copy link
Collaborator Author

You must remove all takes and interest holders from an acquisition file property before removing that property from an acquisition file

comments addressed.

Copy link
Contributor

✅ No secrets were detected in the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants