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

Adds support for searching user type in user creation form #7332

Closed
wants to merge 9 commits into from
Closed

Adds support for searching user type in user creation form #7332

wants to merge 9 commits into from

Conversation

Ashutosh0602
Copy link
Contributor

Proposed Changes

There was difficulty in finding the suitable user type from the options provided because there was large number of options were available.

So, added a search tab in User Type to find out the best desired type easily from the detailed list.
Fixed some type error and UI flaws.

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@Ashutosh0602 Ashutosh0602 requested a review from a team as a code owner March 3, 2024 19:51
Copy link

vercel bot commented Mar 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2024 7:40pm

Copy link

netlify bot commented Mar 3, 2024

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 217eee7
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/65e8c6573dcd230008e9a50a
😎 Deploy Preview https://deploy-preview-7332--care-egov-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines 271 to 278
onResponse: (result) => {
if (!result || !result.res || !result.data) return;
if (userIndex <= USER_TYPES.indexOf("StateAdmin")) {
setStates([authUser.state_object!]);
} else {
setStates(result.data.results);
}
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onResponse: (result) => {
if (!result || !result.res || !result.data) return;
if (userIndex <= USER_TYPES.indexOf("StateAdmin")) {
setStates([authUser.state_object!]);
} else {
setStates(result.data.results);
}
},
onResponse: ({ data }) => {
if (!data) {
return;
}
if (userIndex <= USER_TYPES.indexOf("StateAdmin")) {
setStates([authUser.state_object!]);
} else {
setStates(data.results);
}
},

return (
<Page
title={headerText}
options={
<Link
href="https://school.coronasafe.network/targets/12953"
href="https://school.coronasafe.network/targets/12953 "
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed?

Suggested change
href="https://school.coronasafe.network/targets/12953 "
href="https://school.coronasafe.network/targets/12953"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While saving the file extra space removed.
Is this making any conflict?

className="inline-block rounded border border-gray-600 bg-gray-50 px-4 py-2 text-gray-600 transition hover:bg-gray-100"
target="_blank"
target="_blank "
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed?

Suggested change
target="_blank "
target="_blank"

@@ -631,15 +632,26 @@ export const UserAdd = (props: UserProps) => {
showAll={false}
/>
</div>
<SelectFormField

{/* <SelectFormField
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this instead of commenting it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was checking the everything was working properly I will remove and make new commit.

{...field("user_type")}
label="User Type"
placeholder="Search for a user type..."
// value={userTypes}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// value={userTypes}

@@ -326,7 +327,7 @@ export const UserAdd = (props: UserProps) => {

const setFacility = (selected: FacilityModel | FacilityModel[] | null) => {
setSelectedFacility(selected as FacilityModel[]);
const form = { ...state.form };
const form: any = { ...state.form };
Copy link
Member

Choose a reason for hiding this comment

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

Casting to any not allowed.

Suggested change
const form: any = { ...state.form };
const form = { ...state.form };

@rithviknishad
Copy link
Member

Also, it'd be great if you could give a meaningful title for the PR. It could convey what this PR has done. For example: "Adds support for searching user type in user creation form"

@rithviknishad
Copy link
Member

Also, ensure cypress tests are passing

@Ashutosh0602 Ashutosh0602 changed the title Search option Adds support for searching user type in user creation form Mar 6, 2024
@Ashutosh0602
Copy link
Contributor Author

@rithviknishad I fix the previous mentioned changes.

@rithviknishad
Copy link
Member

Are you sure the cypress tests are passing?

@rithviknishad rithviknishad added changes required cypress failed pull request with cypress test failure labels Mar 7, 2024
@Ashutosh0602
Copy link
Contributor Author

No, 3 out of 7 cypress test failed and rest is passed.
And currently I am trying to figure it out. If you can give some suggestion related to cypress I am new to using cypress

@Ashutosh0602
Copy link
Contributor Author

@rithviknishad Is there something I have to do here?

@rithviknishad
Copy link
Member

Hey, you can connect your FE to your local backend and run the cypress tests by following the documentation here...

https://github.com/coronasafe/care_fe?tab=readme-ov-file#-run-cypress-tests

Once you've identified which tests are failing, you can update the corresponding cypress tests. The test code would be present in the same file name as the cypress test spec name.

Copy link

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 20, 2024
Copy link

Hi, @gigincg, @nihal467, @khavinshankar, @mathew-alex, This pr has been automatically closed because it has not had any recent activity. Thank you for your contributions. Feel free to repopen the pr.

@github-actions github-actions bot closed this Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes required cypress failed pull request with cypress test failure stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue in finding the value in options tab
2 participants