Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
dc5e6f8
Add initial layout for notifications framework
MridulS Jul 22, 2016
3d9a85d
Add spec for share graph event db functions
MridulS Jul 22, 2016
c79c1bd
Add boilerplate test
MridulS Jul 22, 2016
5cd7500
Add error page if user is not logged in for notifications
MridulS Jul 24, 2016
d507c44
Update ShareGraphEvent model and implement functions in db.py
MridulS Jul 28, 2016
1dd3d6b
update login error text
MridulS Jul 28, 2016
47feb76
create new event after sharing graph with group
MridulS Jul 28, 2016
0e9465f
add support for reading notifications
MridulS Jul 31, 2016
6982837
add boilerplate exceptions file
MridulS Aug 1, 2016
ec85ee2
Add functionality to read all events of a group
MridulS Aug 1, 2016
f630f8a
update notifications template
MridulS Aug 5, 2016
4c24e6c
add functionality to view all notifications of a group
MridulS Aug 5, 2016
db07aa5
changes
MridulS Aug 11, 2016
e98ff6f
Merge branch 'master' into notification
MridulS Aug 11, 2016
99c3aff
Update notifications code according to the spec
MridulS Aug 14, 2016
80124a4
update UI for notifications
MridulS Aug 14, 2016
7d5a2c9
removing unnecessary packages
adbharadwaj Aug 16, 2016
24ea0e2
refactoring get all notifications flow-still incomplete
adbharadwaj Aug 16, 2016
3706261
refactored notifications flow
adbharadwaj Aug 16, 2016
9c672dd
Update notifications.js according to notifications spec
MridulS Aug 17, 2016
3542ff8
Add group information and the number badges to the left column in not…
MridulS Aug 17, 2016
8cd3d09
update urls.py to point to correct view
MridulS Aug 17, 2016
d9f31bb
Update views and db to match the notifications spec
MridulS Aug 17, 2016
d4f786a
Remove unnecessary file get-pip
MridulS Aug 17, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion graphs/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
ex. 'id' for user table would be 'user_id'
'''

from sqlalchemy import Column, Integer, String, ForeignKey, Table, Index, ForeignKeyConstraint
from sqlalchemy import Column, Integer, String, ForeignKey, Table, Index, ForeignKeyConstraint, Boolean
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import relationship, backref
from sqlalchemy.types import TIMESTAMP
Expand Down Expand Up @@ -310,6 +310,27 @@ class Edge(Base):
ForeignKeyConstraint([user_id, graph_id, tail_node_id], [Node.user_id, Node.graph_id, Node.node_id], ondelete="CASCADE", onupdate="CASCADE"), {})
#no relationship specified

class share_graph_event(Base):
__tablename__ = 'share_graph_event'
Copy link
Member

Choose a reason for hiding this comment

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

Need some docs on the schema. What does the table store? Why do you need each attribute?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Class names should have camel case.

# unique id for each share graph event
id = Column(Integer, autoincrement=True, primary_key=True)
# id of the graph shared
graph_id = Column(String, ForeignKey('graph.graph_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
# id of the owner of the graph which is shared
owner_id = Column(String, ForeignKey('group.owner_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
# id of the group the graph is shared in
group_id = Column(String, ForeignKey('group.group_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
# id of the member of the group.
# Hence there can be multiple share graph events if a owner shares a grap
# with a group. A share graph event will be created for all the memebers
# of the group except the owner of the graph (the one who shared it).
member_id = Column(String, ForeignKey('user.user_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
# timestamp at which the share graph event occured
share_time = Column(TIMESTAMP, nullable = False)
# Boolean value to track if notifications is read or not.
# if True then the notification is active, i.e not read
is_active = Column(Boolean)
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need a unique constraint on (graph_id, owner_id, group_id, member_id)? In other words, an owner can share a graph with a group-member pair only once?

Are you not adding the unique constraint because you are thinking of someone unsharing a graph and then sharing it again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you not adding the unique constraint because you are thinking of someone unsharing a graph and then sharing it again?

A user can unshare the graph and as discussed in the meeting with Ted we can delete the share event.
But I think the unique constraint part will be a part of the logic implemented in the db.py file.

Copy link
Member

Choose a reason for hiding this comment

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

It should be enforced in the db, especially we *can *enforce it at the
schema level.

On Sun, Jul 24, 2016 at 10:28 AM, Mridul Seth [email protected]
wrote:

In graphs/models.py
#191 (comment)
:

@@ -310,6 +310,16 @@ class Edge(Base):
ForeignKeyConstraint([user_id, graph_id, tail_node_id], [Node.user_id, Node.graph_id, Node.node_id], ondelete="CASCADE", onupdate="CASCADE"), {})
#no relationship specified

+class share_graph_event(Base):

  • tablename = 'share_graph_event'
  • id = Column(Integer, autoincrement=True, primary_key=True)
  • graph_id = Column(String, ForeignKey('graph.graph_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
  • owner_id = Column(String, ForeignKey('group.owner_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
  • group_id = Column(String, ForeignKey('group.group_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
  • member_id = Column(String, ForeignKey('user.user_id', ondelete="CASCADE", onupdate="CASCADE"), nullable=False)
  • share_time = Column(TIMESTAMP, nullable = False)
  • is_active = Column(Boolean)

Are you not adding the unique constraint because you are thinking of
someone unsharing a graph and then sharing it again?

A user can unshare the graph and as discussed in the meeting with Ted we
can delete the share event.
But I think the unique constraint part will be a part of the logic
implemented in the db.py file.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Murali-group/GraphSpace/pull/191/files/c79c1bd7ef35f3349c121b770faa993665cf6d34#r71989505,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGkWUG8U3i4hJ6l_OPDOGCeJCw2w1TSHks5qY3ZwgaJpZM4JS_ZW
.


#Create indices
Index('graph_public_idx', Graph.public)
Index('graph_owner_idx', Graph.user_id)
Expand Down
5 changes: 4 additions & 1 deletion graphs/templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ <h4 class="modal-title" id="myModalLabel">Forgot Information</h4>
{% endif %}
<li><a href="{% url 'features' %}">Features</a></li>
<li><a href="{% url 'help_tutorial' %}">Help</a></li>
<li><a href="{% url 'help_about' %}" style="text-align:center">About Us</a></li>
<li><a href="{% url 'help_about' %}" style="text-align:center">About Us</a></li>
{% if uid != None %}
Copy link
Member

Choose a reason for hiding this comment

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

Is this check sufficient? Should we not check that the user is indeed logged in and is a "valid" user? I am asking without knowledge of how this check is performed in other parts of GraphSpace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this will be enough as uid gets a value only if the user is logged in and a valid user. Logic of this https://github.com/Murali-group/GraphSpace/blob/master/graphs/auth/login.py#L44

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screen shot 2016-07-24 at 4 52 03 pm
The user will get a screen like this if its not a valid log in

<li><a href="{% url 'notifications' %}">Notifications</a></li>
{% endif %}
</ul>

<!-- display login form if not logged in -->
Expand Down
41 changes: 41 additions & 0 deletions graphs/templates/graphs/notifications.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{% extends 'base.html' %}
{% block content %}
{% load staticfiles %}

{% if uid != None %}

<div class="panel panel-primary">
<div class="panel-heading">
<h3 class="panel-title">Group1</h3>
</div>
<table class="table">
<tr>
<td> <a href="">[email protected] shared graph some_name_of_graph with the group Group 1.</td>
Copy link
Member

@tmmurali tmmurali Jul 24, 2016

Choose a reason for hiding this comment

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

Shouldn't there be generic variables here instead of [email protected], the name of the graph, and the name of the group? After reading the rest of the PR, I understand now that you do not have the full implementation and that most of the methods are stubs.

</tr>
<tr>
<td> <a href="">[email protected] shared graph some_other_graph with the group Group 1.</td>
</tr>

</table>
</div>

<div class="panel panel-primary">
<div class="panel-heading">
<h3 class="panel-title">Group 2</h3>
</div>
<table class="table">
<tr>
<td> <a href="">[email protected] shared graph new_graph with the group Group 2.</td>
</tr>
<tr>
<td> <a href="">[email protected] shared graph test_graph with the group Group 2.</td>
</tr>
<tr>
<td> <a href="">[email protected] shared graph random_graph with the group Group 2.</td>
</tr>

</table>

</div>
{% endif %}
{% endblock %}
3 changes: 3 additions & 0 deletions graphs/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
url(r'^graphs/public/$', views.public_graphs, name='public_graphs'),
url(r'^graphs/upload/$', views.upload_graph_through_ui, name='upload_graph_through_ui'),

# notifications page
url(r'^notifications/$', views.notifications, name='notifications'),

# view graph page. This contains regular expression to catch url in the form of the following:
# /graphs/email_address/graph_id/
# regex from http://www.regular-expressions.info/email.html
Expand Down
59 changes: 59 additions & 0 deletions graphs/util/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -4742,3 +4742,62 @@ def constructResponse(statusCode, message, error):
response['Error'] = error

return response

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a comment to document the return value for each method.


def add_share_graph_event(graph_id, owner_id, group_id, member_id):
'''
Add a new share graph event to the table.
After sharing the graph with a group this function will create
a share graph event for all the users in that group

@param graph_id: id of the graph shared
@param owner_id: owner of the graph which is shared
@param group_id: id of the grop
@param member_id: id of the member the graph is shared
'''
pass

def update_share_graph_event(event_id, active):
'''
Update the share graph event. Change its active state.
If active is True then the notification is not read/clicked.

@param event_id: id of the share graph event
@param active: Boolean value, update the state of event
'''
pass


def delete_share_graph_event(event_id, member_id):
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove the member id from this. event id is unique to a share graph event.

'''
Delete the share graph event from the table for the member

@param event_id: id of the share graph event
@param member_id: id of the member

'''
pass


def get_share_graph_event_by_member_id(member_id):
'''
Return all the share graph events for a user
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should say returns list of all the share graph events instead of all the share graph events.


@param member_id: id of the user
'''
pass

def get_share_graph_event_by_id(event_id):
'''
Return share graph event notification

@param event_id: id of the event
'''
pass


def get_all_share_graph_event():
'''
Return all the share graph events.
'''
pass
12 changes: 12 additions & 0 deletions graphs/util/db_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from graphs.models import share_graph_event
from django.test import TestCase
from db import (get_all_share_graph_event, get_share_graph_event_by_id,
get_share_graph_event_by_member_id, delete_share_graph_event,
update_share_graph_event, add_share_graph_event)


class share_graph_event_test(TestCase):
def basic_test(self):
share_event = add_share_graph_event()
# add tests after adding logic

12 changes: 12 additions & 0 deletions graphs/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,18 @@ def _graphs_page(request, view_type):

return render(request, 'graphs/graphs.html', context)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add python documentation for this method. https://www.python.org/dev/peps/pep-0257/

def notifications(request):
# context of the view to be passed in for rendering
context = {}
# handle login
context = login(request)
# Checks to see if a user is currently logged on
uid = request.session['uid']
if uid is None:
context['Error'] = "You need to be logged in to view notifications"
return render(request, 'graphs/error.html', context)
return render(request, 'graphs/notifications.html', context)

def upload_graph_through_ui(request):

if request.method == 'POST':
Expand Down