Tribes profile #5555
Tribes profile #5555
Conversation
|
Is this ready for review or still WIP? |
Codecov Report
@@ Coverage Diff @@
## master #5555 +/- ##
==========================================
+ Coverage 30.25% 30.26% +<.01%
==========================================
Files 247 247
Lines 21085 21143 +58
Branches 3050 3060 +10
==========================================
+ Hits 6380 6398 +18
- Misses 14429 14466 +37
- Partials 276 279 +3
Continue to review full report at Codecov.
|
app/assets/v2/js/open-chat.js
Outdated
|
|
||
| $(elem).on('click', function() { | ||
| let user = $(elem).data('openchat'); | ||
| let url = user ? `https://chat.gitcoin.co/gitcoin/messages/@${user}` : 'https://chat.gitcoin.co/'; |
app/assets/v2/js/open-chat.js
Outdated
| let chatWindow; | ||
|
|
||
| $(elem).on('click', function() { | ||
| let user = $(elem).data('openchat'); |
| @@ -0,0 +1,14 @@ | |||
| const openChat = () => { | |||
| $('[data-openchat]').each(function(index, elem) { | |||
| let chatWindow; | |||
There was a problem hiding this comment.
do we need this variable ?
There was a problem hiding this comment.
not really now is actually so we can close the window from js if we need it
There was a problem hiding this comment.
also because Im pointing to the same window when user open a new chat.
| let user = $(elem).data('openchat'); | ||
| let url = user ? `https://chat.gitcoin.co/gitcoin/messages/@${user}` : 'https://chat.gitcoin.co/'; | ||
|
|
||
| chatWindow = window.open(url, 'Loading', 'top=0,left=0,width=400,height=600,status=no,toolbar=no,location=no,menubar=no,titlebar=no'); |
There was a problem hiding this comment.
we could do just a window.open ! what do you think ?
There was a problem hiding this comment.
what do you mean?
There was a problem hiding this comment.
oo without the variable declaration? Is because I want to reuse the window to avoid multiple popups
app/assets/v2/js/pages/join_tribe.js
Outdated
| $(elem).attr('disabled', true); | ||
|
|
||
| let tribe = $(elem).data('jointribe'); | ||
| let url = `/join/${tribe}/`; |
app/assets/v2/js/pages/join_tribe.js
Outdated
| $(elem).on('click', function() { | ||
| $(elem).attr('disabled', true); | ||
|
|
||
| let tribe = $(elem).data('jointribe'); |
app/assets/v2/js/pages/join_tribe.js
Outdated
| $(elem).text('Leave Tribe'); | ||
| } else { | ||
| $(elem).text('Join Tribe'); | ||
| } |
There was a problem hiding this comment.
response.is_member ? $(elem).text('Leave Tribe') : $(elem).text('Join Tribe') YAY / NAY ?
app/assets/v2/js/pages/join_tribe.js
Outdated
| $(elem).attr('disabled', true); | ||
|
|
||
| let memberId = $(elem).data('tribeleader'); | ||
| let url = 'tribe/tribeleader/'; |
app/assets/v2/js/pages/join_tribe.js
Outdated
| $(elem).on('click', function() { | ||
| $(elem).attr('disabled', true); | ||
|
|
||
| let memberId = $(elem).data('tribeleader'); |
| const activateQuill = () => { | ||
| if (quill) { | ||
| return quill.isEnabled() ? destroyQuill() : rebuildQuill(); | ||
| } |
There was a problem hiding this comment.
we could rewrite this as
return quill && quill.isEnabled() ? destroyQuill() : rebuildQuill(); and drop the if condition
There was a problem hiding this comment.
quill is undefined in the first run that will block the code to continue
There was a problem hiding this comment.
derp derp ! you are right
app/assets/v2/js/pages/tribe-edit.js
Outdated
| $('[data-savetribe]').on('click', function() { | ||
| let tribe = $(this).data('savetribe'); | ||
| let url = `/savetribe/${tribe}/`; | ||
| var sendSave = fetchData ( |
app/assets/v2/js/pages/tribe-edit.js
Outdated
|
|
||
| $('[data-savetribe]').on('click', function() { | ||
| let tribe = $(this).data('savetribe'); | ||
| let url = `/savetribe/${tribe}/`; |
app/assets/v2/js/pages/tribe-edit.js
Outdated
| }; | ||
|
|
||
| $('[data-savetribe]').on('click', function() { | ||
| let tribe = $(this).data('savetribe'); |
app/dashboard/admin.py
Outdated
| list_display = ['pk', 'profile', 'org', 'leader', 'status'] | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
we can remove 1 extra line!
app/dashboard/views.py
Outdated
| @csrf_exempt | ||
| @require_POST | ||
| def save_tribe(request,handle): | ||
| # quill.root.innerHTML |
app/app/urls.py
Outdated
| re_path(r'^tribes', retail.views.tribes, name='tribes'), | ||
| path('join/<str:handle>/', dashboard.views.join_tribe, name='join_tribe'), | ||
| path('savetribe/<str:handle>/', dashboard.views.save_tribe, name='save_tribe'), | ||
| path('tribe/tribeleader/', dashboard.views.tribe_leader, name='tribe_leader'), |
There was a problem hiding this comment.
^ would the URL's be
tribe/<str:handle>/join
tribe/<str:handle>/save
tribes/<str:handle>/tribe-leader
^ thoughts on renaming the URI as such ?
Also since there is a join shouldn't there be a /leave?
Or can we group it into a common URI like
tribe/<str:handle>/ and have a body { join: True } / { join: False}
There was a problem hiding this comment.
yes I like your pattern, actually I had plans to look at it, let me change it real quick.
But about having 2 urls to change true/false I think is not needed.
And about the body they are very diff actually pointing to diff models.
| @@ -1,3 +1,4 @@ | |||
| /* stylelint-disable no-descending-specificity */ | |||
There was a problem hiding this comment.
make sense to add this throughout the site as opposed to just this file ? (not sure where this goes in though )
There was a problem hiding this comment.
yes because that rule is a pain in the ass using tags instead of class names, and for example every rule with img will complain.
app/dashboard/views.py
Outdated
| @csrf_exempt | ||
| @require_POST | ||
| def join_tribe(request, handle): | ||
| print(handle, request.user.profile) |
app/dashboard/views.py
Outdated
| print(handle, request.user.profile) | ||
| if request.user.is_authenticated: | ||
| profile = request.user.profile if hasattr(request.user, 'profile') else None | ||
| print(profile) |
app/dashboard/views.py
Outdated
| @csrf_exempt | ||
| @require_POST | ||
| def join_tribe(request, handle): | ||
| print(handle, request.user.profile) |
app/dashboard/views.py
Outdated
| print(handle, request.user.profile) | ||
| if request.user.is_authenticated: | ||
| profile = request.user.profile if hasattr(request.user, 'profile') else None | ||
| print(profile) |
| if request.user.is_authenticated: | ||
| profile = request.user.profile if hasattr(request.user, 'profile') else None | ||
| print(profile) | ||
| try: |
There was a problem hiding this comment.
Lets not use try / except here - if something goes wrong (like a user doesn't have a profile, or the handle doesn't exist) then its going to jump to the exception handling code. Just use an if statement here with exists to see if the user is a tribe member like if profile and handle and TribeMember.objects.get(profile=profile, ...).exists(): TribeMember.objects.get(...).delete()
There was a problem hiding this comment.
I will get back this to try because is failing in this way
File "/code/app/dashboard/views.py", line 4057, in join_tribe web_1 | if profile and handle and TribeMember.objects.get(profile=profile, org__handle__iexact=handle).exist(): web_1 | dashboard.models.TribeMember.DoesNotExist: TribeMember matching query does not exist.
| 'success': True, | ||
| 'is_member': False, | ||
| }, | ||
| status=200) |
There was a problem hiding this comment.
^ do we need to pass success True ? cause status is 200
There was a problem hiding this comment.
yes more a rest pattern
Description
Add tribes and actions
Refers/Fixes
Fix #5516
Testing