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

System log 查詢介面相關 後端 #61

Merged
merged 3 commits into from
Jun 11, 2019

Conversation

FWcloud916
Copy link
Collaborator

#40 System log 查詢介面相關
add : 操作類別查詢 api
add : 操作紀錄查詢 api

Method URI
GET api/system-log
GET api/system-log-type

Copy link
Collaborator

@hashman hashman left a comment

Choose a reason for hiding this comment

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

LGTM

@hashman
Copy link
Collaborator

hashman commented May 31, 2019

我看錯 change 了,我晚點繼續看

Copy link
Collaborator

@hashman hashman left a comment

Choose a reason for hiding this comment

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

問題發想,會需要 system_log_type 的 api 嗎?

app/SystemLog.php Outdated Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Show resolved Hide resolved
@hashman
Copy link
Collaborator

hashman commented May 31, 2019

麻煩請 @FWcloud916 先 rebase develop 後繼續進行開發修改,這樣才可以拿到 Mars 的 change 如果有問題都可以提出,謝謝

#58

@FWcloud916
Copy link
Collaborator Author

我好像越整理越亂了@@

@FWcloud916
Copy link
Collaborator Author

FWcloud916 commented Jun 1, 2019

問題發想,會需要 system_log_type 的 api 嗎?

個人觀點:
如果想要使用者增加 log 的類型,並且增加一個可以在後台對每個功能修改 log 內容跟 log 類型,可能才比較有意義,因為現在 log 都是 hardcode 加上去的,即使今天 log 的類型增加了,要修改還是要直接修改程式碼

@hashman
Copy link
Collaborator

hashman commented Jun 1, 2019

問題發想,會需要 system_log_type 的 api 嗎?

如果想要使用者增加 log 的類型,並且增加一個可以在後台對每個功能修改 log 內容跟 log 類型 才會有意義.

我們先將它單純化好了,先用最簡單的方式完成,未來如果有這個需求,我們再補上去,你覺得呢?

@FWcloud916
Copy link
Collaborator Author

問題發想,會需要 system_log_type 的 api 嗎?

如果想要使用者增加 log 的類型,並且增加一個可以在後台對每個功能修改 log 內容跟 log 類型 才會有意義.

我們先將它單純化好了,先用最簡單的方式完成,未來如果有這個需求,我們再補上去,你覺得呢?

問題發想,會需要 system_log_type 的 api 嗎?

如果想要使用者增加 log 的類型,並且增加一個可以在後台對每個功能修改 log 內容跟 log 類型 才會有意義.

我們先將它單純化好了,先用最簡單的方式完成,未來如果有這個需求,我們再補上去,你覺得呢?

好的

@FWcloud916 FWcloud916 force-pushed the feature/LogViewBackend branch 3 times, most recently from b1a148d to 442e56f Compare June 5, 2019 15:49
@FWcloud916
Copy link
Collaborator Author

FWcloud916 commented Jun 5, 2019

@hashman 我先改這樣 麻煩看一下
我會再試試使用 relation 取代 join 謝謝!

@hashman
Copy link
Collaborator

hashman commented Jun 7, 2019

先麻煩 @FWcloud916 針對 commit 的建議內容,另外是也幫忙 rebase 最新的 develop branch 謝謝

@FWcloud916 FWcloud916 force-pushed the feature/LogViewBackend branch 3 times, most recently from d1a8429 to 9e39a83 Compare June 7, 2019 03:18
@FWcloud916
Copy link
Collaborator Author

@hashman 好了 謝謝!

@FWcloud916 FWcloud916 force-pushed the feature/LogViewBackend branch 3 times, most recently from 66fcc81 to 46389ee Compare June 7, 2019 03:36
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
routes/web.php Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
@FWcloud916 FWcloud916 force-pushed the feature/LogViewBackend branch 3 times, most recently from 997754d to 2a0f603 Compare June 9, 2019 11:49
@FWcloud916
Copy link
Collaborator Author

@hashman 加上測試了,麻煩幫我看一下,謝謝

Copy link
Collaborator

@hashman hashman left a comment

Choose a reason for hiding this comment

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

comment 的內容再麻煩 @FWcloud916 看看囉,測試的方式是正確的沒問題, Controller 沒有使用到的 function 先不要寫上去,還有些 coding style 的問題,我就沒有一一列上去了,再麻煩一併調整

謝謝

app/Http/Controllers/SystemLogTypeController.php Outdated Show resolved Hide resolved
app/SystemLogType.php Outdated Show resolved Hide resolved
routes/web.php Show resolved Hide resolved
tests/Feature/Controllers/SystemLogContorllerTest.php Outdated Show resolved Hide resolved
tests/Feature/Controllers/SystemLogContorllerTest.php Outdated Show resolved Hide resolved
app/Http/Controllers/SystemLogController.php Outdated Show resolved Hide resolved
@FWcloud916
Copy link
Collaborator Author

@hashman 好了

Copy link
Collaborator

@hashman hashman left a comment

Choose a reason for hiding this comment

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

這個 PR 我想應該讓你成長不少,希望你也在這個 PR 中獲得很多,花了多少時間東西就是你的,謝謝你的幫忙囉

@hashman hashman merged commit 2cb6d59 into MOPCON:develop Jun 11, 2019
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.

None yet

3 participants