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

추천/비추천/추천삭제 기능추가. #140

Merged
merged 13 commits into from Jan 23, 2016

Conversation

bjrambo
Copy link
Member

@bjrambo bjrambo commented Jan 21, 2016

#118 이슈 에서 일딴 추천만 캔슬할 수 잇는 기능 추가 했습니다.

추가적으로 계속코드 추가할 예정입니다.
지금은..피곤해성 ㅠㅠ..

@bjrambo
Copy link
Member Author

bjrambo commented Jan 21, 2016

아..그리고 스킨의 라이선스가 LGPL이던데, 이것도 라이믹스 코어에 종속되는 스킨이니 라이선스를 GPL v2 으로 변경해뒀습니다. 문제가 있다면 알려주세용 ~

@kijin kijin added the new feature 새 기능 (PR에서만 사용) label Jan 21, 2016
@kijin
Copy link
Member

kijin commented Jan 22, 2016

deleteDocumentVotedLog 쿼리에 document_srl만 넣고 실행하면 현재 회원의 추천뿐 아니라 그 문서의 모든 추천기록이 증발합니다. member_srl도 넣을 수 있도록 XML쿼리 정의를 수정할 필요가 있어 보입니다.

getVoted() 메소드가 페이지마다 2번씩 실행되는데, 매번 쿼리를 실행하도록 되어 있어서 느려질 것 같습니다. 현재 사용자의 추천/비추천 내역이 $_SESSION['voted_document'][$document_srl]에 캐싱되고 있으니 이 변수가 존재하는 경우 쿼리 없이 그대로 반환하는 것이 좋겠습니다.

그런데 $_SESSION['voted_document'][$document_srl]에는 truefalse만 저장되기 때문에 추천한 건지 비추천한 건지 구분할 수가 없습니다. 지금은 추천 부분만 작업하셨지만 비추천에도 똑같이 적용하려면 구분이 필요하겠죠? true 대신 추천은 1, 비추천은 -1을 저장하는 것을 제안합니다. 조건문에서는 둘 다 true로 처리되기 때문에 다른 자료와의 호환성을 유지할 수 있습니다. (추천 취소시 false는 그대로 유지해도 되고, 0으로 해도 됩니다.)

@bjrambo
Copy link
Member Author

bjrambo commented Jan 22, 2016

$_SESSION['voted_document'][$document_srl] 를 사용할때 비추천시에는
$_SESSION['blamed_count'][$document_srl] 을 사용하도록 해보겠습니다.

@kijin
Copy link
Member

kijin commented Jan 22, 2016

updateVotedCount() 메소드에서는 이미 추천과 비추천 구분 없이 $_SESSION['voted_document']만 쓰고 있어요. 초전역 변수가 많아질수록 디버깅하기 귀찮으니 가능하면 하나만 쓰죠 ㅎㅎ

@bjrambo
Copy link
Member Author

bjrambo commented Jan 22, 2016

@kijin 추천과 비추천을 나눌 필요가 있을까요?
생각해보니, 추천을 하고나서 다시 비추천을 누르게된다면, 해당 세션값이 달라질거같은데요.
결론적으로 -1값이 되었던, 1이 되었던, 해당 게시판에 추천 또는 비추천을 한 사실이 잇음 없음 정도로만 기록 해줘도 되지않을까 생각되는데.. 이 부분은 어떻게 생각하시나요?

어차피 캔슬시에는 false으로 반환될거고요.

다만, SESSION을 이용하여 지속적인 추천, 취소를 막기위해서
`$_SESSION['voted_cancel'][$document_srl] 에 하나씩 카운트를 하도록 하여 횟수를 지정하도록 하고, 해당 취소 카운트를 몇번 취소하는지 결정하도록 하는편은 어떤지 제안해봅니다.

@kijin
Copy link
Member

kijin commented Jan 22, 2016

@qw5414 스킨에서 추천취소단추를 보여줄지 비추취소단추를 보여줄지 결정하기 위해서는 예전에 추천했는지 비추했는지 쉽게 알 수 있어야 합니다. 기본스킨에서는 구분하지 않을지도 모르지만, 스킨에 따라서는 매우 중요한 기능이 될 수도 있어요.

포인트만 정확하게 계산해 준다면 추천했다가 취소했다가 다시 추천하는 행위를 계속 반복하더라도 서버 부하 이외의 문제는 없을 거예요. 서버 부하 유발은 굳이 추천을 반복하지 않더라도 F5 키만 계속 눌러주면 얼마든지 유발할 수 있으니 추천 액션에서 막는다고 어차피 막히는 것도 아니고요.

@bjrambo
Copy link
Member Author

bjrambo commented Jan 22, 2016

@kijin 방금 커밋한 내용 봐주세용~~

@kijin
Copy link
Member

kijin commented Jan 22, 2016

@qw5414 문자열 반환은 불필요합니다. 숫자로 해야 > 0인지 0인지 < 0인지 구분하기 쉬워요. 추천과 비추를 꼭 1점씩만 한다는 법도 없으니 (예: 회원 레벨에 따라 추천에 가중치를 부여하는 기능?) 실제 추천한 분량을 정확하게 반환하도록 합시다.

@@ -64,7 +64,7 @@
</li>
</ul>
<script>
var sTitle = '{str_ireplace(array('<script', '</script'), array("<scr'+'ipt", "</scr'+'ipt"), addslashes($oDocument->getTitleText()))}';
var sTitle = '{$oDocument->getTitleText()}';
Copy link
Member Author

Choose a reason for hiding this comment

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

이거 때문에 도저히.. ㅠㅠ 그냥 지워둬도 괜찮을것 같아요(SNS제목 띄울껀데 -0-.)

<action name="procDocumentVoteDown" type="controller" />
<action name="procDocumentVoteDownCancel" type="controller" />
Copy link
Member Author

Choose a reason for hiding this comment

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

이거.. 때문에 짜증나느데..ㅠ.ㅠ 그냥 전부 탭으로 밀어버릴까요?
이게 자꾸 탭이 이상하게 인식하네요 ㅡ,.ㅡ

@kijin
Copy link
Member

kijin commented Jan 22, 2016

#150 추천/비추천/신고 모듈과 혹시 충돌하는 부분이 없는지 확인 부탁드립니다. 특히 "레벨별 추천개수 조정기능" 사용시 추천을 취소하면 어떻게 되는지 점검이 필요합니다. 5점을 추천하고 1점만 취소한다든지...

@bjrambo
Copy link
Member Author

bjrambo commented Jan 23, 2016

@kijin 별개로 동작하는 모듈이네요.
저희와는 상관없지만, display트리거를 사용하여, 버튼을 남겨노아 자신들의 액션을 docall시키는 형태로 제작되어있는것 같습니다.
procVoteVoteDown
그리고 확인해보니.. 메소드 역시 자신들걸로 사용하고 있엇고요..

@bjrambo
Copy link
Member Author

bjrambo commented Jan 23, 2016

충돌적인 요소나 포인트 요소는 좀 더 확인하고.. 제작해볼께요.

function getVoted()
{
if(!$this->document_srl) return;
if($_SESSION['voted_document'][$this->document_srl] == '1')
Copy link
Member

Choose a reason for hiding this comment

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

이거 조건식을 더 간단하게

if($_SESSION['voted_document'][$this->document_srl])
{
    return $_SESSION['voted_document'][$this->document_srl];
}

이렇게 하면 될 듯... (아래 391~398줄도 마찬가지)

Copy link
Member Author

Choose a reason for hiding this comment

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

어.... 나에겐 왜 이런 머리가 없는거지

@@ -1201,11 +1294,15 @@ function updateVotedCount($document_srl, $point = 1)
if($point < 0)
{
$args->blamed_count = $oDocument->get('blamed_count') + $point;
// Leave in the session information
$_SESSION['voted_document'][$document_srl] = -1;
Copy link
Member

Choose a reason for hiding this comment

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

여기랑 아랫줄도 그냥 $point를 넣으면 됩니다.

bjrambo pushed a commit that referenced this pull request Jan 23, 2016
추천/비추천/추천삭제 기능추가.
@bjrambo bjrambo merged commit d5c921b into rhymix:develop Jan 23, 2016
@bjrambo bjrambo deleted the pr/document-voted branch February 5, 2016 05:51
@onepang
Copy link

onepang commented Feb 24, 2016

그런데 일반적으로 추천을 취소할때는 비추천을 누르는게 아니라
눌렀던 추천을 한번더 누르지 않나요?

구글 플러스 좋아요나 페이스북 좋아요와 마찬가지로

@bjrambo
Copy link
Member Author

bjrambo commented Feb 24, 2016

생각하신대로 한번 더 눌러서 취소가 될 수 있도록 되어있습니다.

@onepang
Copy link

onepang commented Feb 24, 2016

@bjrambo
er

저같은경우 추천을 한번 더 클릭 하였을시 이렇게 반응 됩니다.

@bjrambo
Copy link
Member Author

bjrambo commented Feb 24, 2016

이 PR에 담겨져있는 파일중에,
board/skins/~~~~~으로 되어있는 스킨 파일을 보시고 사용하시는 스킨에 적용해보시기 바래요~

여러이슈에서 이러한 게시판적인 부분을 라이믹스에서 지원하지 못해서 Sketchbook 같은 대중적인 스킨의 경우 라이믹스로 같이 진행해보자고 의견 모으고 있지만, 현재 바로 들어가는건 힘들것 같아서 해당 스킨 합치는 작업은 보류하고 있습니다.

@kijin kijin mentioned this pull request Apr 24, 2016
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature 새 기능 (PR에서만 사용)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants