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.

Sign up for GitHub

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

Jump to bottom

21812-Basic-Staff-Review-ContIn-Section #2893

Merged
merged 7 commits into from
Jul 8, 2024
Merged

21812-Basic-Staff-Review-ContIn-Section #2893

merged 7 commits into from
Jul 8, 2024

Conversation

ArwenQin
Copy link
Contributor

@ArwenQin ArwenQin commented Jul 4, 2024 •
edited by JazzarKarim
Loading

Issue #:
bcgov/entity#21812

Description of changes:

  • Implemented the new Staff Dashboard section - Continuation Application table
  • This is the minimum table implementation. Sorting and filtering are in other ticket.
  • Table with paging in footer, and displaying items
  • 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 sbc-auth license (Apache 2.0).

ArwenQin
ArwenQin commented Jul 4, 2024
View reviewed changes
auth-web/src/services/business.services.ts
// const url = `${ConfigHelper.getLegalAPIV2Url()}/businesses/continuation-in`
// const response = await axios.get(url)
// return response.data
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mock some sample response here. The url is to be updated when "get reviews" endpoint is available

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right. Waiting for bcgov/entity#21929.

ArwenQin
ArwenQin commented Jul 4, 2024
View reviewed changes
auth-web/src/components/auth/staff/continuation-in/StaffContinuationInTable.vue Outdated
</tr>

<!-- Second row has search boxes. Search and filter to be implemented -->
<tr class="header-row-2 mt-2 px-2">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The search and filter is not implemented yet

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right. Will be done in bcgov/entity#21983.

@ArwenQin
Copy link
Contributor Author

ArwenQin commented Jul 4, 2024 •
edited
Loading

Updated the Review and View button, now:
image

@severinbeauvais severinbeauvais self-requested a review July 4, 2024 18:06
@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jul 4, 2024 •
edited
Loading

Just to confirm: do we want them in different length?

I'd say no, and your updated screenshot with equal-size buttons looks great! see comment below

@severinbeauvais
Copy link
Collaborator

Just to confirm: do we want them in different length?

I'd say no, and your updated screenshot with equal-size buttons looks great!

Change of decision: Andy says make them different lengths as in the design.

severinbeauvais
severinbeauvais reviewed Jul 4, 2024
View reviewed changes
auth-web/src/services/business.services.ts Outdated
@@ -93,4 +93,37 @@ export default class BusinessService {
`${ConfigHelper.getLegalAPIV2Url()}/businesses/${businessIdentifier}/documents/summary`, config)
CommonUtils.fileDownload(data?.data, `${businessIdentifier} Summary.pdf`, data?.headers['content-type'])
}

static async fetchContinuationBusinesses (): Promise<any> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You are not fetching businesses. You are fetching (continuation) reviews.

Chat with Paul and Ketaki about what they're going to name the endpoints (re: line 125) and name this method accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just left this issue open: I will update the endpoint name when Paul gets back to office.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. Maybe you can update this when you work on bcgov/entity#21068.

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jul 4, 2024 •
edited
Loading

Please fix the issue reference in the description.

Do you have any unit tests planned?

@ArwenQin
Copy link
Contributor Author

ArwenQin commented Jul 4, 2024 •
edited by severinbeauvais
Loading

Please fix the issue reference in the description.

Do you have any unit tests planned?

Thank you! I fixed the issues and added unit tests.
Just one issue open: the fetch Review endpoint name - I will update it when Paul gets back to office.

@bcgov bcgov deleted a comment from bcregistry-sre Jul 4, 2024
@ArwenQin
Copy link
Contributor Author

ArwenQin commented Jul 8, 2024 •
edited
Loading

Hi Severin, if we want to merge by 1pm today:
I fixed the issues. Could you please review, thanks!
Just one issue open: the fetch Review endpoint name - still wait for Paul to get back to office.
The language support, per our chat, I think Naveen and Andy will discuss? So I left it unchanged for now.

@severinbeauvais
Copy link
Collaborator

Arwen, please rebase.

severinbeauvais
severinbeauvais approved these changes Jul 8, 2024
View reviewed changes
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.

LGTM. If there are small changes, they can be done in the next ticket. (But you absolutely need to rebase.)

@Jxio , @rodrigo-barraza , could you please review?

@severinbeauvais
Copy link
Collaborator

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Jul 8, 2024 •
edited by severinbeauvais
Loading

Temporary Url for review: https://bcregistry-account-dev--pr-2893-78331f5w.web.app

SB says, try this: https://bcregistry-account-dev--pr-2893-78331f5w.web.app/staff/dashboard/active

@bcgov bcgov deleted a comment from bcregistry-sre Jul 8, 2024
@ArwenQin ArwenQin force-pushed the 21812-Basic-Staff-Review-Section branch from 1ea5922 to c4953af Compare July 8, 2024 21:28
severinbeauvais
severinbeauvais reviewed Jul 8, 2024
View reviewed changes
auth-web/src/components/auth/staff/continuation-application/ContinuationApplicationTable.vue
@@ -0,0 +1,321 @@
<template>
<div id="continuation-application-review-table">
<v-form class="continuation-application-search continuation-application-search">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicate classes.

Fix this in next ticket.

severinbeauvais
severinbeauvais reviewed Jul 8, 2024
View reviewed changes
auth-web/src/components/auth/staff/continuation-application/ContinuationApplications.vue
components: {
ContinuationApplicationTable
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fix indent in future ticket.

@severinbeauvais severinbeauvais merged commit e4c54a5 into bcgov:main Jul 8, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@severinbeauvais severinbeauvais severinbeauvais approved these changes

@Jxio Jxio Awaiting requested review from Jxio

@seeker25 seeker25 Awaiting requested review from seeker25

@ochiu ochiu Awaiting requested review from ochiu

@rodrigo-barraza rodrigo-barraza Awaiting requested review from rodrigo-barraza

Assignees

@ArwenQin ArwenQin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ArwenQin @severinbeauvais @bcregistry-sre