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

Show CA "Needs attention" #201

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Show CA "Needs attention" #201

merged 3 commits into from
Dec 13, 2024

Conversation

IanVS
Copy link
Contributor

@IanVS IanVS commented Dec 5, 2024

Fixes #191

This makes Android behave more like iOS, wherein it is not possible to save an invalid site. As a bonus here, we'll display a more useful error message as well, referencing why the form cannot be saved.

This fixes a bug to correctly mark CA as "Needs attention" if it is empty (and the site is not managed) ref: #202

To test:
- In Android, create a new site, give it a name, and save. You should get an error message.
- Then fill out the certificate and CA, and again save. It should still properly save the site.

Open question:

  • In iOS, it's possible to save a site without a CA, but it does require a host cert. Is that our desired behavior, or should we require a valid config before saving?

Answer: we want to avoid requiring a valid config. Much of this PR has been removed for now, only leaving the CA needs attention fix.

nbrownus
nbrownus previously approved these changes Dec 5, 2024
Copy link
Contributor

@nbrownus nbrownus left a comment

Choose a reason for hiding this comment

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

For both iOS and Android it would be ideal to be able to not lose work if you can't complete a fully valid configuration in one run.

@IanVS
Copy link
Contributor Author

IanVS commented Dec 5, 2024

I'm going to back out some of this then, and work on showing a warning prompt that can be accepted to save an invalid config.

@IanVS IanVS changed the title Do not save site if there are errors Show CA "Needs attention" Dec 10, 2024
@IanVS IanVS merged commit c97732d into main Dec 13, 2024
4 checks passed
@IanVS IanVS deleted the save-prompt branch December 13, 2024 15:47
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.

Clicking save on an unvalidated form closes the form without saving
3 participants