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

14691 COLIN states mapping to LEAR RFC #14968

Merged
merged 14 commits into from
Feb 3, 2023

Conversation

argush3
Copy link
Collaborator

@argush3 argush3 commented Jan 13, 2023

Issue #: #14691

Description of changes:

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

@argush3 argush3 self-assigned this Jan 13, 2023
@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch 2 times, most recently from c2fff9f to 6946fa5 Compare January 13, 2023 15:15
@argush3 argush3 marked this pull request as ready for review January 13, 2023 15:16
@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 6946fa5 to 0e22ac4 Compare January 13, 2023 20:34
Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

My first thought was that the new way appears more complicated than the COLIN way. However, I really like the idea of managing "allowable transitions". When implemented, the new approach is likely to be easier to use.

@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 16e16b7 to f28126d Compare January 18, 2023 17:35
@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 4299165 to 4c44465 Compare January 18, 2023 19:17
@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 4c44465 to c8a02eb Compare January 18, 2023 19:22
Copy link
Collaborator

@thorwolpert thorwolpert left a comment

Choose a reason for hiding this comment

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

we should kick this around a bit more.
rc chat threads aren't going to get us there, even though this is really close.
thanks for gathering this.

rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

Are we still pursuing the concept of sub-state?

@argush3
Copy link
Collaborator Author

argush3 commented Jan 25, 2023

Are we still pursuing the concept of sub-state?

I don't think so.

We will have things like good standing, warnings(compliance, missing business info) and other flow statuses to give us what we need.

@severinbeauvais
Copy link
Collaborator

Are we still pursuing the concept of sub-state?

I don't think so.

We will have things like good standing, warnings(compliance, missing business info) and other flow statuses to give us what we need.

How will we know things like Frozen, or in various dissolution states?

@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 80e622d to 10db0d2 Compare January 26, 2023 16:58
@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 10db0d2 to bb73aa1 Compare January 26, 2023 17:04
@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from bb73aa1 to 4a69571 Compare January 26, 2023 17:06
Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

Looks workbable to me 👍

@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from e12d811 to fccd173 Compare January 26, 2023 17:14
@argush3
Copy link
Collaborator Author

argush3 commented Jan 26, 2023

@severinbeauvais A note about the frozen flag. There is already an adminFreeze flag passed back by the BE on the business object. This can be used.

@severinbeauvais
Copy link
Collaborator

@severinbeauvais A note about the frozen flag. There is already an adminFreeze flag passed back by the BE on the business object. This can be used.

Thanks. Travis is looking into this as well (so Auth Web knows). FYI.

rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@thorwolpert thorwolpert left a comment

Choose a reason for hiding this comment

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

We seem pretty close and narrowing in on smaller details.
Do we want to fully work it out, or if we're pretty close get it in to start trying it?

rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
rfcs/rfc-colin-states-mapping-to-lear.md Outdated Show resolved Hide resolved
rfcs/rfc-colin-states-mapping-to-lear.md Show resolved Hide resolved
@severinbeauvais
Copy link
Collaborator

If we're worried about the volume of data, we could move this part of the response to an actions sub-endpoint, or allow for a ?verbose=minimum flag to allow the caller to reduce.

In a business response object, I think a "large" amount of data is OK.

In a filings response object, I'd prefer to see minimum data returned, and the UI can make another call to fetch more data when the user needs it. But I don't know how this would work for the API telling us "you're allowed to correct this item" (which is business logic currently in the UI). I also don't know how the API will inform the UI whether, say, the business can be dissolved (which is more business logic in the UI)... though this flag/rule could be in the business response object.

@argush3 argush3 force-pushed the 14691_colin_states_mapping_to_lear_rfc branch from 0864202 to 85e97d6 Compare February 3, 2023 15:11
Copy link
Collaborator

@thorwolpert thorwolpert left a comment

Choose a reason for hiding this comment

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

As far as I can see, this captures all of our discussions, learnings from the business and pulling the design into a way we can keep valid tests and scenarios in the CI pipeline.
I like it.

@argush3 argush3 merged commit ae8c6a3 into bcgov:main Feb 3, 2023
@argush3 argush3 deleted the 14691_colin_states_mapping_to_lear_rfc branch February 3, 2023 18:11
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.

None yet

6 participants