Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Validate claimed facility website field and show as hyperlink #647

Merged
merged 5 commits into from
Jul 9, 2019

Conversation

jwalgran
Copy link
Contributor

@jwalgran jwalgran commented Jul 3, 2019

Overview

Adds client validation of the facility claim website field, changes the related model field to a URLField, and makes valid URLs clickable on the facility details sidebar.

Connects #587

Demo

Form validation

Screen Shot 2019-07-02 at 4 56 40 PM

Clickable link

Screen Shot 2019-07-02 at 4 56 17 PM

Testing Instructions

Assumes that the fixtures have been loaded and processed.

  • Run ./scripts/update
  • Make sure facility claiming is enabled
    • ./scripts/manage waffle_switch claim_a_facility on
    • Restart ./scripts/server
  • Log in as c2@example.com
  • Claim a facility
    • On the "Facility Information" step, verify that you can only proceed to the next step if the website field is either empty or a valid url (with or without a scheme)
  • Log in as c1@example.com and approve the claim via http://localhost:6543/dashboard/claims
  • Log back in as c2@example.com and open the facility claim details page via http://localhost:6543/claimed
  • Verify that you can only enter valid URLs into the website field (protocol is optional)
    • Verify that the save button is disabled until the website field is valid
  • Enter a valid URL in the website field and save the form.
  • Browse the facility details page and verify that the website is a clickable link that opens the URL in a new tab/window.

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@rajadain
Copy link
Contributor

rajadain commented Jul 3, 2019

We should probably add similar validation on this page:

image

which currently allows free text input.

@jwalgran
Copy link
Contributor Author

jwalgran commented Jul 3, 2019

Thanks you for catching that. I overlooked the fact that there are two separate website fields, one for claim research and one for public display on the facility page. Will fix.

@rajadain
Copy link
Contributor

rajadain commented Jul 3, 2019

And should we also add similar validation in the profile page:

image

or wait until a later issue for it?

@rajadain
Copy link
Contributor

rajadain commented Jul 3, 2019

I also noticed that when I had a tab previously open at the search home page, searching for the facility and going to its details page did not update with the claim (bottom), but the details page on a full page refresh did (top):

image

Is that a known issue? If not, I'll make a fresh one for it.

@jwalgran
Copy link
Contributor Author

jwalgran commented Jul 3, 2019

should we also add similar validation in the profile page

I think so. No need to make a separate issue.

I changed my mind about this. It is out of scope of this claim a facility specific issue. I will make a new issue for it.

searching for the facility and going to its details page did not update with the claim...Is that a known issue? If not, I'll make a fresh one for it.

I don't think this is a known issue and yes please make a fresh one for it.

This field should only ever contain valid URLs.
Ensures that the field shows a validation error and the save button is disabled
unless the website field is either blank or a valid URL.
We are validating that the values submitted for this field are valid URLs so we
can safely make them into links. This follows the pattern of the website field
on the contributor profile page.
Ensure that if a value is entered in the field it is a valid URL. We reuse and
update the existing validation used on the facility profile form by moving the
validation function to `utils.js`.
@jwalgran
Copy link
Contributor Author

jwalgran commented Jul 9, 2019

I added validation to the claim a facility wizard in c5d77ff

I changed my my about also including changes to the user/contributor profile page. It is out of scope of this claim a facility specific issue. I created #656 for it.

This is rebased on develop and ready for re-review.

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

Taking another look now.

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

When there is an error in the "Official Name" field, in addition to the "Some required fields are missing or invalid" message in the bottom, the field itself has a red highlight:

image

However, when the "Facility Website" field has invalid data, it stays blue, instead of red:

image

Making it red would help identify which field has the invalid data.

@kellyi
Copy link
Contributor

kellyi commented Jul 9, 2019

re the URL field on the ClaimFacility...Step file, I believe you can just add a line like this one -- which I used to validate that the LinkedIn URL was a real URL -- to have the field be outlined in red when it has an error state:

https://github.com/open-apparel-registry/open-apparel-registry/pull/650/files#diff-cca727c1524411105c81408bc15d1331R85

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

When viewing a Pending facility claim as an admin, the website is not clickable, even when valid:

image

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

After a claim is approved, information submitted in the claim (such as website, contact person name, etc) are not pre-filled in the Claimed Facility Details page:

image

Is this expected / is there a separate issue for this?

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

The red highlight works correctly on the Claimed Facility Details page:

image

@kellyi
Copy link
Contributor

kellyi commented Jul 9, 2019

information submitted in the claim (such as website, contact person name, etc) are not pre-filled in the Claimed Facility Details page

This one is intentional. The Claim Form is only for evaluating the claim; there are separate fields for the public profile info. They are separate so that the claim form info remains non-public & is not subsequently editable by the claimant.

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

Link works correctly from the UI once saved in the Claimed Facility Details form:

2019-07-09 15 26 40

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

I also noticed that when I had a tab previously open at the search home page, searching for the facility and going to its details page did not update with the claim (bottom), but the details page on a full page refresh did (top)

I am unable to reproduce this right now, was probably doing something wrong during testing. Going to ignore it for now.

@rajadain
Copy link
Contributor

rajadain commented Jul 9, 2019

The two things this needs for approval:

  • Make the website textbox be red when the URL is incorrect in the Claim application form
  • Make the website clickable when reviewing a claim as an admin

Copy link
Contributor

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested, this works well! @kellyi has claimed the final two tasks for #650, so this can be merged without them.

@jwalgran
Copy link
Contributor Author

jwalgran commented Jul 9, 2019

Thanks for the thorough review

@jwalgran jwalgran merged commit 974da2f into develop Jul 9, 2019
@jwalgran jwalgran deleted the jcw/validate-facility-website branch July 9, 2019 19:57
kellyi pushed a commit that referenced this pull request Jul 9, 2019
Make some changes suggested in
#647 (comment)
to:

- outline an invalid website string in red on the claim form
- make the claim's submitted website clickable when displayed on the
admin dashboard

Also use the addProtocolToWebsiteURLIfMissing function on the LinkedIn
url displayed on the dashboard.
kellyi pushed a commit that referenced this pull request Jul 9, 2019
Make some changes suggested in
#647 (comment)
to:

- outline an invalid website string in red on the claim form
- make the claim's submitted website clickable when displayed on the
admin dashboard

Also use the addProtocolToWebsiteURLIfMissing function on the LinkedIn
url displayed on the dashboard.
kellyi pushed a commit that referenced this pull request Jul 9, 2019
Make some changes suggested in
#647 (comment)
to:

- outline an invalid website string in red on the claim form
- make the claim's submitted website clickable when displayed on the
admin dashboard

Also use the addProtocolToWebsiteURLIfMissing function on the LinkedIn
url displayed on the dashboard.
kellyi pushed a commit that referenced this pull request Jul 10, 2019
Make some changes suggested in
#647 (comment)
to:

- outline an invalid website string in red on the claim form
- make the claim's submitted website clickable when displayed on the
admin dashboard

Also use the addProtocolToWebsiteURLIfMissing function on the LinkedIn
url displayed on the dashboard.
kellyi pushed a commit that referenced this pull request Jul 11, 2019
Make some changes suggested in
#647 (comment)
to:

- outline an invalid website string in red on the claim form
- make the claim's submitted website clickable when displayed on the
admin dashboard

Also use the addProtocolToWebsiteURLIfMissing function on the LinkedIn
url displayed on the dashboard.
kellyi pushed a commit that referenced this pull request Jul 11, 2019
Make some changes suggested in
#647 (comment)
to:

- outline an invalid website string in red on the claim form
- make the claim's submitted website clickable when displayed on the
admin dashboard

Also use the addProtocolToWebsiteURLIfMissing function on the LinkedIn
url displayed on the dashboard.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants