onweru / hugo-swift-theme

A simple open source theme for publishing with hugo
https://neuralvibes.com/
Other
120 stars 64 forks source link

Staticman submission error #35

Closed VincentTam closed 5 years ago

VincentTam commented 5 years ago

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.

onweru commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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]":"dfe@er.p","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 commented 5 years ago

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]":"dfe@er.p","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 commented 5 years ago

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 commented 5 years ago

There is virtually no documentation on staticman V3

I've once written a list of documentation pages in https://github.com/eduardoboucas/staticman/issues/294#issuecomment-517617378. 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. https://github.com/eduardoboucas/staticman/issues/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 :computer:! :smiley_cat:

VincentTam commented 5 years ago

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: 'mail@eduardoboucas.com',
  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 ! :v: (upcoming nested comments, once I've put the above :bulb: solution into code. :man_technologist: )
onweru commented 5 years ago

Sounds good

VincentTam commented 5 years ago

@onweru My PR #36 will automatically resolve this.

VincentTam commented 5 years ago

Closed via #36.