-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix: Developer menu apps refresh bug #488
Conversation
* fix: Categories query and create btn bug * fix: Add role desc, app version desc bug
Codecov Report
@@ Coverage Diff @@
## master #488 +/- ##
==========================================
- Coverage 17.62% 17.58% -0.04%
==========================================
Files 90 90
Lines 1396 1399 +3
Branches 290 293 +3
==========================================
Hits 246 246
- Misses 1094 1097 +3
Partials 56 56
Continue to review full report at Codecov.
|
server/routes/api.js
Outdated
@@ -92,6 +80,11 @@ router.post('/api/*', async ctx => { | |||
Object.assign(chooseToken, res); | |||
} | |||
|
|||
// todo: redirect is no effect | |||
if (!chooseToken.access_token) { | |||
ctx.redirect('/login?url=' + endUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ctx.redirect
will redirect current request, meaning /api/apps
=> /api/login
. You can use ctx.throw(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, change to: ctx.throw(401, 'Unauthorized: invalid access token'); but still not implement page redirection when not found token.
src/pages/Admin/Overview/index.jsx
Outdated
const { isLoading } = appStore; | ||
const lng = i18n.language || 'zh'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to pass lng
params, getPastTime
can read it from localStorage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
src/utils/index.js
Outdated
const now = new Date(); | ||
const date = new Date(time); | ||
const diff = (now.getTime() - date.getTime()) / (60 * 60 * 1000); | ||
return diff / 24 > 1 ? parseInt(diff / 24) + ' days ago' : parseInt(diff) + ' hours ago'; | ||
let daysago = ' 天前', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think put locale string in this function is not good. What if there are other locales ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, add a translation.js for js file word translation.
fix: Categories query and create btn bug
fix: Add role desc, app version desc bug
fix: Judge API transfer token bug