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

Fixes ISO 8601 timestamp validation #490

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sgg
Copy link

@sgg sgg commented Apr 15, 2020

The regex used to perform timestamp validation fails to parse valid
timestamps that do not have fractional seconds (e.g. 2020-04-14T19:54:46Z).

This change (1) updates the regex to account for this and (2) moves the regex
into a global variable so it lives in one place.

The new regex is sourced from Ch 4.7 in Regular Expressions Cookbook 2e
by Goyvaerts and Levithan.

note - From what I can tell Postman only allows strings in environment
variables so I cannot simply put the validation function there.

@abonander
Copy link

Opened #832 for this issue because I didn't see this PR. It looks like it needs rebased though.

The regex used to perform timestamp validation fails to parse valid
timestamps that do not have fractional seconds (e.g. `2020-04-14T19:54:46Z`).

This change (1) updates the regex to account for this and (2) moves the regex
into a global variable so it lives in one place.

The new regex is sourced from Ch 4.7 in `Regular Expressions Cookbook` 2e
by Goyvaerts and Levithan.

note - From what I can tell Postman only allows strings in environment
variables so I cannot simply put the validation function there.
@netlify
Copy link

netlify bot commented Dec 27, 2021

Deploy Preview for realworld-docs ready!

Name Link
🔨 Latest commit b7054cd
🔍 Latest deploy log https://app.netlify.com/sites/realworld-docs/deploys/628a4ccfacbc2800089ac394
😎 Deploy Preview https://deploy-preview-490--realworld-docs.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 settings.

@geromegrignon
Copy link
Contributor

Hey @abonander, I rebased the PR. Could you review the changes by running the JSON file on your project?

@abonander
Copy link

abonander commented Jan 28, 2022

@geromegrignon Sorry for the delay. I pulled the branch but I am unable to run the tests and have them actually point at my local instance. I try running with APIURL=http://localhost:8080 ./run-api-tests.sh but it still points to api.realworld.io:

> APIURL=http://localhost:8080/api ./run-api-tests.sh
+++ dirname ./run-api-tests.sh
++ cd .
++ pwd
+ SCRIPTDIR=/home/abonander/realworld/api
+ APIURL=http://localhost:8080/api
+ USERNAME=abonander
+ [email protected]
+ PASSWORD=password
+ npx newman run /home/abonander/realworld/api/Conduit.postman_collection.json --delay-request 500 --global-var APIURL=http://localhost:8080/api --global-var USERNAME=abonander --global-var [email protected] --global-var PASSWORD=password
newman

Conduit

❏ Auth
↳ Register
  POST https://api.realworld.io/api/users [422 Unprocessable Entity, 879B, 403ms]
  1. Response contains "user" property
  2. User has "email" property
  3. User has "username" property
  4. User has "bio" property
  5. User has "image" property
  6. User has "token" property

Strangely enough, however, this doesn't seem to occur on main. Bad rebase, maybe?

Comment on lines 1952 to 1931
"variable": [
{
"key": "APIURL",
"value": "https://api.realworld.io/api"
},
{
"key": "USERNAME",
"value": "ggrignon1234"
},
{
"key": "PASSWORD",
"value": "123456"
},
{
"key": "EMAIL",
"value": "[email protected]"
}
]
Copy link

@abonander abonander Jan 28, 2022

Choose a reason for hiding this comment

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

@geromegrignon looks like this is overriding the values passed in by the script.

Copy link
Contributor

Choose a reason for hiding this comment

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

@abonander sorry i was away from the project for a while, I fixed it in case you still have some interest in this project :)

@geromegrignon geromegrignon force-pushed the sgg/postman-timestamp-validation branch from c832b7f to b7054cd Compare May 22, 2022 14:46
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