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

postToMap.sh checks #1106

Merged
merged 5 commits into from
Mar 29, 2022
Merged

postToMap.sh checks #1106

merged 5 commits into from
Mar 29, 2022

Conversation

EricClaeys
Copy link
Collaborator

  • Make sure Website URL and Image URL start with "http:" or "https:". Two testers didn't do this, and it breaks the link on the map.

**Please check this on your Pi. I have not tested it since I don't currently have access to my Pi. **
Simply copy the script to the Pi, then in the WebUI, prepend an "x" to your Website URL and Image URL and save the changes. You should see two error messages at the top of the page. Then remove the "x" and save the changes. It should work.

* If Website or Image URL is given, the must begin with "http:" or "https:", otherwise clicking on them in the map fails.
* In messages, use the WebUI labels, not variable names which users won't ever see.
* Fix missing double quote (found thanks to shellcheck!)
* Combine 3 lines into one to make it easier to read.
@thomasjacquin
Copy link
Collaborator

@EricClaeys
Thanks for fixing it. I noticed it too and I was thinking about fixing it server side by prepending the protocol but then I realized I would have to know if it was http or https. I think it's better to educated people by telling them the format we expect in that field.

I have tested your change and it works in the command line (after fixing the typo) but it doesn't throw any error message in the WebUI.

image

image

@EricClaeys
Copy link
Collaborator Author

EricClaeys commented Mar 28, 2022 via email

@thomasjacquin
Copy link
Collaborator

Typos: See annotations in the code review.

I can enter empty fields and the WebUI is fine with that.

@EricClaeys
Copy link
Collaborator Author

@thomasjacquin Does your /etc/raspap/camera_options_ZWO.json file have checkchanges in it? I would guess not, since I haven't updated the .json files yet so they aren't performing checks on any changed fields.
I'll do a PR for that what I get to my observatory and have access to my Pi.

I don't see any annotations - just your comment from 25 minutes ago.

You can go ahead and approve the PR since I know what the issue is.

@thomasjacquin
Copy link
Collaborator

@EricClaeys No there's no checkchanges field.

It's weird it's not showing up on your side. I'm seeing it near the top of that PR.
image

@EricClaeys
Copy link
Collaborator Author

@thomasjacquin Are you at #1106 when you see the annotations? That's where I am. Maybe you see them because you're the owner?

Just fixed the missing "s". Thanks for catching that.

@linuxkidd
Copy link
Collaborator

@thomasjacquin You fell into a trap I have hit a few times as well. Those comments on the code are 'pending'... you still have to submit them, otherwise no one else will see them. :)

@thomasjacquin
Copy link
Collaborator

@thomasjacquin You fell into a trap I have hit a few times as well. Those comments on the code are 'pending'... you still have to submit them, otherwise no one else will see them. :)

Thanks @linuxkidd. I’ve never used that feature before. I’ll make sur to submit next time

fi

# website_url and image_url are optional
# website_url and image_url are optional, but need to start with "http:" or "https:" if present
if [ "${WEBSITE_URL}" != "" ] && [ "${WEBSITE_URL:0:5}" != "http:" ] && [ "${WEBSITE_URL:0:6}" != "http:" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you meant "${WEBSITE_URL:0:6}" != "https:" ]

echo -e "${ERROR_MSG_START}ERROR: 'Website URL' must begin with 'http:' or 'https:'.${ERROR_MSG_END}"
OK=false
fi
if [ "${IMAGE_URL}" != "" ] && [ "${IMAGE_URL:0:5}" != "http:" ] && [ "${IMAGE_URL:0:6}" != "http:" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as line 133.

@linuxkidd linuxkidd merged commit 28a156e into master Mar 29, 2022
@EricClaeys EricClaeys deleted the postToMap.sh-checks branch March 29, 2022 23:10
@EricClaeys
Copy link
Collaborator Author

@thomasjacquin You fell into a trap I have hit a few times as well. Those comments on the code are 'pending'... you still have to submit them, otherwise no one else will see them. :)

Yea!! My eyesight is still ok!

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.

3 participants