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

22169 - api specs notice of withdrawal #3136

Merged

Conversation

eason-pan-bc
Copy link
Collaborator

@eason-pan-bc eason-pan-bc commented Dec 18, 2024

Issue #: /bcgov/entity#22169

Description of changes:
Adding items listed below to the api specs

  • Notice of Withdrawal Schema
  • Sample request
  • Sample responses

Specs Render Screenshots:

  • Overview
    image
  • Sample Request
    image
  • Sample Request with Options
    image
  • Sample Response - successful
    image
  • Sample Response - 400
    image
  • Sample Response - 401
    image
  • Sample Response - 404
    image

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).

@eason-pan-bc eason-pan-bc self-assigned this Dec 18, 2024
@eason-pan-bc eason-pan-bc marked this pull request as ready for review December 18, 2024 19:27
@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Dec 18, 2024

Shouldn't it be 404 if not found? thanks for the fix

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Dec 18, 2024

What about the optional data that could be posted / fetched? Like the 2 checkbox values and the court order. thanks for the fix

@eason-pan-bc

This comment was marked as resolved.

@eason-pan-bc

This comment was marked as resolved.

@severinbeauvais
Copy link
Collaborator

Shouldn't it be 404 if not found?

Good point, we will need to adjust the validation code a bit to make it happen. It's kinda a straight forward change, should I change it and include it in this PR/ticket.

How about: update the spec now and update the code in one of the existing Legal API tickets?

@eason-pan-bc
Copy link
Collaborator Author

Shouldn't it be 404 if not found?

Good point, we will need to adjust the validation code a bit to make it happen. It's kinda a straight forward change, should I change it and include it in this PR/ticket.

How about: update the spec now and update the code in one of the existing Legal API tickets?

Sounds good, added this task to 24989

@ArwenQin
Copy link
Collaborator

ArwenQin commented Dec 18, 2024

I would like to confirm the schema for the two checkbox values. Should they be placed within a sub-object or not?

"noticeOfWithdrawal": {
    "filingId": "153004",
    "courtOrder": {
        "fileNumber": "TT0010045",
        "effectOfOrder": "planOfArrangement"
    },
    "planOfArrangement": {
          "partOfPoa": true,
          "hasTakenEffect": true
   }
}

Or

"noticeOfWithdrawal": {
    "filingId": "153004",
    "courtOrder": {
        "fileNumber": "TT0010045",
        "effectOfOrder": "planOfArrangement"
    },
   "partOfPoa": true,
   "hasTakenEffect": true
}

@eason-pan-bc
Copy link
Collaborator Author

eason-pan-bc commented Dec 18, 2024

"noticeOfWithdrawal": { "filingId": "153004", "courtOrder": { "fileNumber": "TT0010045", "effectOfOrder": "planOfArrangement" }, "planOfArrangement": { "partOfPoa": true, "comeIntoEffect": true } }

Or "noticeOfWithdrawal": { "filingId": "153004", "courtOrder": { "fileNumber": "TT0010045", "effectOfOrder": "planOfArrangement" }, "partOfPoa": true, "comeIntoEffect": true }

I'm a bit confused, I only saw courtOrder as part of the NoW schema. https://github.com/bcgov/business-schemas/blob/main/src/registry_schemas/schemas/notice_of_withdrawal.json#L21C11-L21C122

For planOfArrangement, do you mean effectOfOrder in courtOrder https://github.com/bcgov/business-schemas/blob/5038428a5ae6f8247083478ab8b13ef9eb1be11b/src/registry_schemas/schemas/court_order.json#L29-L33?

And for courtOrder schema, it's been the same for 2 years https://github.com/bcgov/business-schemas/blob/main/src/registry_schemas/schemas/court_order.json
Is there a NoW schema update? @severinbeauvais @ArwenQin

@ArwenQin
Copy link
Collaborator

Yes, the schema may need to be updated, as there are 2 new check boxes properties for NoW.
For now, We are not sure whether we need to include the 2 properties, and whether in a sub-object.
Severin and I discussed, for now, we say Yes

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Dec 18, 2024

Is there a NoW schema update? @severinbeauvais @ArwenQin

Not yet. The courtOrder object is still there, and the 2 new boolean values may be optional or used for UI only. Still undecided but we don't need to add those to the schema yet.

Update: Yes we will use the 2 new properties. I prefer them in the root of the noticeOfWithdrawal object.

@eason-pan-bc
Copy link
Collaborator Author

eason-pan-bc commented Dec 18, 2024

Not yet. The courtOrder object is still there, and the 2 new boolean values may be optional or used for UI only. Still undecided but we don't need to add those to the schema yet.

Update: Yes we will use the 2 new properties. I prefer them in the root of the noticeOfWithdrawal object.

Since the api specs follow the schemas, how about I add the option of courtOrder for now. If we ended up updating the NoW schema, we can always come back to update the specs?

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Dec 18, 2024

@ArwenQin Speaking of these 2 new properties, I think I was talking with Mihai and we were unsure whether we should save them or not.

On the one hand, staff should know that if either value is True then the NoW cannot be filed, so they won't even try.

On the other hand, if this filing ever becomes self-serve (ie, not staff-only) then we would use these values right in the UI to prevent the NoW filing.

Please follow up with Mihai and Jacqueline on this. If staff don't need the properties then there's no need to save them. If regular users won't be allowed to file a NoW when either propery is True then, again, there's no need to save them.

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Dec 18, 2024

Since the api specs follow the schemas, how about I add the option of courtOrder for now. If we ended up updating the NoW schema, we can always come back to update the specs?

Sure. The courtOrder object is optional and we haven't decided about the 2 new properties yet. If they turn out to be for UI use only then they should not be part of the schema/spec.

@eason-pan-bc
Copy link
Collaborator Author

added the sample request with options, and updated the filing not found to 404

docs/business.yaml Outdated Show resolved Hide resolved
@eason-pan-bc eason-pan-bc merged commit d69c7dc into bcgov:main Dec 23, 2024
4 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