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

Staticman submission error #35

Closed
VincentTam opened this issue Aug 13, 2019 · 10 comments
Closed

Staticman submission error #35

VincentTam opened this issue Aug 13, 2019 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@VincentTam
Copy link
Collaborator

Screenshot from 2019-08-13 11-22-35

Tested with v1.0.0 at https://framagit.org/staticman-gitlab-pages/hugo-swift-theme/tree/builtv1/. The parent of the head of the branch builtv1 is the Git SHA-1 hash for v1.0.0.

The form to JSON method isn't correct, a field with empty key "":"Comment" is created.

Besides, an OPTIONS type is sent to Staticman API server, which is unexpected.

@VincentTam VincentTam self-assigned this Aug 13, 2019
@VincentTam VincentTam added the bug Something isn't working label Aug 13, 2019
@onweru
Copy link
Owner

onweru commented Aug 13, 2019

I'm using fetch api for this. I suspect that my implementation is somewhat off. I will be looking into the same.

The form to JSON method isn't correct, a field with empty key "":"Comment" is created.
Besides, an OPTIONS type is sent to Staticman API server, which is unexpected.

Thank you for this 👆 diagnosis

onweru pushed a commit that referenced this issue Aug 13, 2019
@onweru
Copy link
Owner

onweru commented Aug 13, 2019

Besides, an OPTIONS type is sent to Staticman API server, which is unexpected.

Maybe the api has changed. Prior versions of staticman accepted a options[slug] value.

@VincentTam
Copy link
Collaborator Author

VincentTam commented Aug 13, 2019

Update: Searching 'staticman fetch "application/json"', I've found this:

      const fields = {
        fields: {
          'fields[name]': this.inputs[0],
          'fields[telphone]': this.inputs[1],
          'fields[classroom]': this.classroom,
          'fields[sex]': this.sex,
          'fields[introduce]': this.introduce,
          'fields[college]': this.college
        }
      }
      const urls = 'https://api.staticman.net/v2/entry/QueenYoung/zerostudio/master/'

      fetch(urls, {
        method: 'POST',
        body: JSON.stringify(fields),
        mode: 'cors',
        headers: {
          'Content-Type': 'application/json'
        }
      })

source: https://github.com/thoamsy/zerostudio/blob/b697337f97005ef6d820683df97919daa7c3d803/src/components/my-form.vue#L128-L147

When a web browser parses this, it reads something like

» console.log(JSON.stringify(fields))
{"fields":{"fields[name]":123,"fields[telphone]":456,"fields[classroom]":458,"fields[sex]":"dftg","fields[introduce]":"introduce","fields[college]":"this.college"}}

See the fields above fields[xxx]? Compare this with the request payload generated by our theme.

{"options[slug]":"4f1f56186d2fece779068cd43c2d5eb2","fields[replyThread]":"","fields[replyID]":"","fields[replyName]":"","fields[name]":"abc","fields[email]":"[email protected]","fields[comment]":"skf.woelkj"}

The API server can't see the top-level field, and it gave a MISSING_PARAMS error. Using this error message, we find the relevant section of Staticman's source code in server.js.

StaticmanAPI.prototype.requireParams = function (params) {
  return function (req, res, next) {
    let missingParams = []


    params.forEach(param => {
      if (
        objectPath.get(req.query, param) === undefined &&
        objectPath.get(req.body, param) === undefined
      ) {
        missingParams.push(param)
      }
    })


    if (missingParams.length) {
      return res.status(500).send({
        success: false,
        errorCode: 'MISSING_PARAMS',
        data: missingParams
      })
    }


    return next()
  }
}

source: https://github.com/eduardoboucas/staticman/blob/2be29f73fa0688e02770b9f118288b6e0e617b01/server.js#L149-L172

The last if statement shows that the value of the data key in API's response is the missingParams. It's a single-element array ["fields"]. I didn't understand this from Staticman's source code until I played with the above example, which makes use of applications/json. That's the first time I've seen this request content type.

I've spent a day on testing. Even though it has not yet been finished, I'll take a rest and resume my work this evening.

@onweru
Copy link
Owner

onweru commented Aug 13, 2019

Update: Searching 'staticman fetch "application/json"', I've found this:

      const fields = {
        fields: {
          'fields[name]': this.inputs[0],
          'fields[telphone]': this.inputs[1],
          'fields[classroom]': this.classroom,
          'fields[sex]': this.sex,
          'fields[introduce]': this.introduce,
          'fields[college]': this.college
        }
      }
      const urls = 'https://api.staticman.net/v2/entry/QueenYoung/zerostudio/master/'

      fetch(urls, {
        method: 'POST',
        body: JSON.stringify(fields),
        mode: 'cors',
        headers: {
          'Content-Type': 'application/json'
        }
      })

source: https://github.com/thoamsy/zerostudio/blob/b697337f97005ef6d820683df97919daa7c3d803/src/components/my-form.vue#L128-L147

When a web browser parses this, it reads something like

» console.log(JSON.stringify(fields))
{"fields":{"fields[name]":123,"fields[telphone]":456,"fields[classroom]":458,"fields[sex]":"dftg","fields[introduce]":"introduce","fields[college]":"this.college"}}

See the fields above fields[xxx]? Compare this with the request payload generated by our theme.

{"options[slug]":"4f1f56186d2fece779068cd43c2d5eb2","fields[replyThread]":"","fields[replyID]":"","fields[replyName]":"","fields[name]":"abc","fields[email]":"[email protected]","fields[comment]":"skf.woelkj"}

The API server can't see the top-level field, and it gave a MISSING_PARAMS error. Using this error message, we find the relevant section of Staticman's source code in server.js.

StaticmanAPI.prototype.requireParams = function (params) {
  return function (req, res, next) {
    let missingParams = []


    params.forEach(param => {
      if (
        objectPath.get(req.query, param) === undefined &&
        objectPath.get(req.body, param) === undefined
      ) {
        missingParams.push(param)
      }
    })


    if (missingParams.length) {
      return res.status(500).send({
        success: false,
        errorCode: 'MISSING_PARAMS',
        data: missingParams
      })
    }


    return next()
  }
}

source: https://github.com/eduardoboucas/staticman/blob/2be29f73fa0688e02770b9f118288b6e0e617b01/server.js#L149-L172

The last if statement shows that the value of the data key in API's response is the missingParams. It's a single-element array ["fields"]. I didn't understand this from Staticman's source code until I played with the above example, which makes use of applications/json. That's the first time I've seen this request content type.

I've spent a day on testing. Even though it has not yet been finished, I'll take a rest and resume my work this evening.

The problem isn't fetch api really. I'm able to construct a valid json object from the form. My problem is that I don't know what values the staticman expects. There is virtually no documentation on staticman V3

@onweru
Copy link
Owner

onweru commented Aug 13, 2019

I seem to be hitting this 429 response ... I suppose I have to host my own instance of staticman with heroku like you've suggested on the readme.

Huh 🤔 , I can relate to this sentiment

Your response here also makes sense. And finally aha.

Perhaps we should direct folks to host their own instance because the public instance will most likely always exceed requests quota.

@VincentTam
Copy link
Collaborator Author

There is virtually no documentation on staticman V3

I've once written a list of documentation pages in eduardoboucas/staticman#294 (comment). Recently, Huginn theme has a section Staticman will illustrate guidelines for configuring Staticman on GitHub, GitLab and Framagit.

I seem to be hitting this 429 response ... I suppose I have to host my own instance of staticman with heroku like you've suggested on the readme.

Heroku isn't the only choice. Other hosting guides:

  1. How to setup Staticman with GitHub and Zeit Now eduardoboucas/staticman#291
  2. https://www.gabescode.com/staticman/2019/01/03/create-staticman-instance.html
  3. https://team-parallax.com/use-selfhosted-staticman-with-gitlab-1
  4. https://www.flyinggrizzly.net/2017/12/setting-up-staticman-server/

Perhaps we should direct folks to host their own instance because the public instance will most likely always exceed requests quota.

Welcome to the world of distributed 💻! 😺

@onweru onweru pinned this issue Aug 13, 2019
@VincentTam
Copy link
Collaborator Author

I've overlooked the commit title "还是不行?" (Still not working?) in the above code block: https://github.com/thoamsy/zerostudio/blob/b697337f97005ef6d820683df97919daa7c3d803/src/components/my-form.vue#L128-L147 After a quick glance over that repo's commit history, I thought that the example above is not so reliable, and I jumped to Staticman's official repo. The following unit test code block gives a big hint to the right request body.

const fields = {
  name: 'Eduardo Bouças',
  email: '[email protected]',
  url: 'https://eduardoboucas.com',
  message: 'This is a sample comment'
}

Another example: https://github.com/eduardoboucas/staticman/blob/30636040d06c64139de6dceb77c2fe32207fa981/test/unit/lib/Notification.test.js#L18-L30.

Recall, from Staticman PR 219's author, that to properly test Staticman, an API client like Postman is recommended, so here's the test results in my minimal Jekyll + Staticman demo: https://git.io/smdemo.

Screenshot from 2019-08-13 18-00-16

Huh , I can relate to this sentiment

Let's be optimistic due to recent developements/improvements of Staticman integrations in several projects, like

  1. Minimal Mistakes (UI improvement)
  2. Beautiful Jekyll (to which I ported MM's SM integration)
  3. Introduction (PR 119 is yet to come in >= v4.2)
  4. Huginn (recently published to Hugo Themes directory, improvement of an abandonned work)
  5. Beautiful Hugo (the theme that I'm actually using for my blog, see the dev branch on GitLab)
  6. Hugo Future Imperfect Slim (another repo that I collaborate with others, recently tested its reCAPTCHA support)
  7. This Theme ! ✌️ (upcoming nested comments, once I've put the above 💡 solution into code. 👨‍💻 )

@onweru
Copy link
Owner

onweru commented Aug 13, 2019

Sounds good

@VincentTam
Copy link
Collaborator Author

@onweru My PR #36 will automatically resolve this.

@VincentTam
Copy link
Collaborator Author

Closed via #36.

@VincentTam VincentTam unpinned this issue Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants