Skip to content

Refactor lean-analytics.swig to use REST API to avoid conflicts with valine#381

Merged
LEAFERx merged 4 commits intotheme-next:masterfrom
LEAFERx:master
Aug 10, 2018
Merged

Refactor lean-analytics.swig to use REST API to avoid conflicts with valine#381
LEAFERx merged 4 commits intotheme-next:masterfrom
LEAFERx:master

Conversation

@LEAFERx
Copy link
Contributor

@LEAFERx LEAFERx commented Aug 9, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes have been added (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs have been added / updated (for bug fixes / features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Issue Number(s): #376

Does this PR introduce a breaking change?

  • Yes.
  • No.

@LEAFERx LEAFERx requested a review from sli1989 August 9, 2018 15:14
url: `https://api.leancloud.cn/1.1${url}`,
headers: {
'X-LC-Id': "{{theme.leancloud_visitors.app_id}}",
'X-LC-Key': "{{theme.leancloud_visitors.app_key}}",
Copy link
Member

@Raincal Raincal Aug 9, 2018

Choose a reason for hiding this comment

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

建议使用 X-LC-Sign 来代替 X-LC-Key
https://leancloud.cn/docs/rest_api.html#hash-1794863242

Copy link

Choose a reason for hiding this comment

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

这一点在保证使用 https 的情况下倒不是必须的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

md5需要引入额外的库 我觉得在https下安全性是没有问题的 而且其实apikey是暴露的 也没有必要

var Counter = function (method, url, data) {
return $.ajax({
method: method,
url: `https://api.leancloud.cn/1.1${url}`,
Copy link
Member

Choose a reason for hiding this comment

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

api.leancloud.cn 这个域名在某些地区无法解析,建议换成应用专属的二级域名
https://forum.leancloud.cn/t/leancloud-api/16234

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已更换

$element.find('.leancloud-visitors-count').text(counter.time + 1);
})
{% endif %}
.fail(function (error) {
Copy link

Choose a reason for hiding this comment

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

这个 error 是 XHR 层面的异常了,LeanCloud 业务层面的异常还需要再拆一层,异常 response body 结构是这样的:

{"code":119,"error":"该操作已被禁止,请变更应用选项『数据存储』。"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改~

} else if ($('.post-title-link').length > 1) {
showTime(Counter);
}
$.get('https://app-router.leancloud.cn/2/route?appId=' + "{{theme.leancloud_visitors.app_id}}")
Copy link
Member

Choose a reason for hiding this comment

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

其实只要把 xxx.api.lncld.net 前缀改成 APPID.slice(0, 8).toLowerCase() 就行了,没必要多发一次请求,就是不知道以后会不会变 @leeyeh

Copy link

@leeyeh leeyeh Aug 10, 2018

Choose a reason for hiding this comment

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

还是有必要的,因为华东节点、美国节点用的域名不是 lncld.net。使用 app-router 机制动态获取域名也是为了避免之前写死了 api.leancloud.cn 被污染后没有任何办法的问题。

Copy link
Member

Choose a reason for hiding this comment

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

了解了~

@LEAFERx LEAFERx requested a review from Raincal August 10, 2018 05:40
@LEAFERx
Copy link
Contributor Author

LEAFERx commented Aug 10, 2018

@Raincal 如果可以请approve或直接merge

Copy link
Member

@Raincal Raincal left a comment

Choose a reason for hiding this comment

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

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Aug 10, 2018

@Raincal 已解决

@LEAFERx LEAFERx merged commit 3a921ce into theme-next:master Aug 10, 2018
@sli1989 sli1989 added this to the v6.4.1 milestone Sep 2, 2018
Jona-lee pushed a commit to Jona-lee/hexo-theme-next that referenced this pull request Oct 10, 2018
…valine (theme-next#381)

* Refactor lean-analytics.swig to use REST API to avoid conflicts with valine

* update api url

* fix error message

* distinguish index and post
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
…valine (theme-next#381)

* Refactor lean-analytics.swig to use REST API to avoid conflicts with valine

* update api url

* fix error message

* distinguish index and post
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.

4 participants