PR#1158 Merged Show user watch list in the index page of the user

Proposed 11 months ago by vivekanand1101
Modified 10 months ago
From forks/vivekanand1101/pagure watch_page  into pagure master
file changed

 1 @@ -2946,3 +2946,44 @@ 

 2                   return True

 3   

 4       return False

 5 + 

 6 + 

 7 + def user_watch_list(session, user):

 8 +     ''' Returns list of all the projects which the user is watching '''

 9 + 

10 +     user_obj = search_user(session, username=user)

11 +     if not user_obj:

12 +         return []

13 + 

14 +     unwatched = session.query(

15 +         model.Watcher

16 +     ).filter(

17 +         model.Watcher.user_id == user_obj.id

18 +     ).filter(

19 +         model.Watcher.watch == False

20 +     )

21 + 

22 +     unwatched_list = []

23 +     if unwatched:

24 +         unwatched_list = [unwatch.project for unwatch in unwatched.all()]

25 + 

26 +     watched = session.query(

27 +         model.Watcher

28 +     ).filter(

29 +         model.Watcher.user_id == user_obj.id

30 +     ).filter(

31 +         model.Watcher.watch == True

32 +     )

33 + 

34 +     watched_list = []

35 +     if watched:

36 +         watched_list = [watch.project for watch in watched.all()]

37 + 

38 +     user_projects = search_projects(session, username=user_obj.user)

39 +     watch = set(watched_list + user_projects)

40 + 

41 +     for project in user_projects:

42 +         if project in unwatched_list:

43 +             watch.remove(project)

44 + 

45 +     return sorted(list(watch), key=lambda proj: proj.name)
file changed

 1 @@ -189,6 +189,34 @@ 

 2               </div>

 3               {% endfor %}

 4           </div>

 5 +         <div class="card" id="watch_list">

 6 +             <div class="card-header">

 7 +                 My Watch List <span class="label label-default">{{ watch_list | count }}</span>

 8 +             </div>

 9 +             {% for repo in watch_list %}

10 +               <div class="list-group">

11 +                 {% if repo.is_fork %}

12 +                   <a class="list-group-item" href="{{ url_for(

13 +                     'view_repo', username=repo.user.username, repo=repo.name) }}">

14 +                     <div class=""><strong><span class="oi" data-glyph="fork">

15 +                             </span> &nbsp;{{ repo.user.username }}/{{ repo.name }}</strong>

16 +                     </div>

17 +                   </a>

18 +                 {% else %}

19 +                   <a class="list-group-item" href="{{ url_for(

20 +                     'view_repo', repo=repo.name) }}">

21 +                     <div class=""><strong><span class="oi" data-glyph="document">

22 +                           </span> &nbsp;{{ repo.name }}</strong>

23 +                     </div>

24 +                   </a>

25 +                 {% endif %}

26 +               </div>

27 +             {% else %}

28 +             <div class="card-block">

29 +                 <p>No project in watch list</p>

30 +             </div>

31 +             {% endfor %}

32 +         </div>

33         </div>

34       </div>

35     </div>
file changed

 1 @@ -116,12 +116,17 @@ 

 2           fork=True,

 3           count=True)

 4   

 5 +     watch_list = pagure.lib.user_watch_list(

 6 +         SESSION,

 7 +         user=flask.g.fas_user.username)

 8 + 

 9       return flask.render_template(

10           'index_auth.html',

11           username=flask.g.fas_user.username,

12           user=user,

13           forks=forks,

14           repos=repos,

15 +         watch_list=watch_list,

16           repopage=repopage,

17           forkpage=forkpage,

18           repos_length=repos_length,
file changed

 1 @@ -91,7 +91,36 @@ 

 2               self.assertEqual(

 3                   output.data.count('<p>No group found</p>'), 1)

 4               self.assertEqual(

 5 -                 output.data.count('<div class="card-header">'), 3)

 6 +                 output.data.count('<div class="card-header">'), 4)

 7 + 

 8 +     def test_watch_list(self):

 9 +         ''' Test for watch list of a user '''

10 + 

11 +         user = tests.FakeUser(username='pingou')

12 +         with tests.user_set(pagure.APP, user):

13 +             output = self.app.get('/')

14 +             self.assertIn(

15 +                 '<div class="text-xs-center">You have no projects</div>',

16 +                 output.data)

17 +             self.assertIn(

18 +                 '<p>You have no forks</p>',

19 +                 output.data)

20 +             self.assertIn(

21 +                 '<p>No project in watch list</p>',

22 +                 output.data)

23 + 

24 +             tests.create_projects(self.session)

25 + 

26 +             output = self.app.get('/')

27 +             self.assertIn(

28 +                 'My Projects <span class="label label-default">2</span>',

29 +                 output.data)

30 +             self.assertIn(

31 +                 'My Forks <span class="label label-default">0</span>',

32 +                 output.data)

33 +             self.assertIn(

34 +                 'My Watch List <span class="label label-default">2</span>',

35 +                 output.data)

36   

37       def test_view_users(self):

38           """ Test the view_users endpoint. """
file changed

 1 @@ -2483,6 +2483,40 @@ 

 2           self.assertFalse(watch)

 3   

 4   

 5 +     def test_user_watch_list(self):

 6 +         ''' test user watch list method of pagure.lib '''

 7 + 

 8 +         tests.create_projects(self.session)

 9 + 

10 +         # He should be watching

11 +         user = tests.FakeUser()

12 +         user.username = 'pingou'

13 +         watch_list_objs = pagure.lib.user_watch_list(

14 +             session=self.session,

15 +             user='pingou',

16 +         )

17 +         watch_list = [obj.name for obj in watch_list_objs]

18 +         self.assertEqual(watch_list, ['test', 'test2'])

19 + 

20 +         # He isn't in the db, thus not watching anything

21 +         user.username = 'vivek'

22 +         watch_list_objs = pagure.lib.user_watch_list(

23 +             session=self.session,

24 +             user='vivek',

25 +         )

26 +         watch_list = [obj.name for obj in watch_list_objs]

27 +         self.assertEqual(watch_list, [])

28 + 

29 +         # He shouldn't be watching anything

30 +         user.username = 'foo'

31 +         watch_list_objs = pagure.lib.user_watch_list(

32 +             session=self.session,

33 +             user='foo',

34 +         )

35 +         watch_list = [obj.name for obj in watch_list_objs]

36 +         self.assertEqual(watch_list, [])

37 + 

38 + 

39   if __name__ == '__main__':

40       SUITE = unittest.TestLoader().loadTestsFromTestCase(PagureLibtests)

41       unittest.TextTestRunner(verbosity=2).run(SUITE)
11 months ago

rebased

11 months ago

rebased

farhaan commented on line 20 of pagure/lib/__init__.py.
11 months ago
(Show)
You should use something like `unwatch` in place of `i`. `i` doesn't convey anything.
farhaan commented on line 32 of pagure/lib/__init__.py.
11 months ago
(Show)
here!
farhaan commented on line 7 of pagure/ui/app.py.
11 months ago
(Show)
I don;t know if its a good idea to make other people see who is watching which all projects.

I actually have this should we have a panel on the side where list of watched projects are mentioned , do we need a different page all together ?

vivekanand1101 commented on line 7 of pagure/ui/app.py.
11 months ago
(Show)
I can see the to-do list of pingou [here](https://pagure.io/user/pingou/requests) so, i think it should be fine
11 months ago

rebased

I actually have this should we have a panel on the side where list of watched projects are mentioned , do we need a different page all together ?

It will become difficult when the number of projects becomes bigger.

Yeah, i would lean against adding a new page for this -- adding them to the user's home page is probably the way to go.

That said, the homepage could do with some rework where we (perhaps) move these all into seperate pages (my repos, forks, watched repos, groups, etc) but currently, these are all on the user home page (i.e. pagure.io when logged in), so we should keep the watched list there too for now.

11 months ago

rebased

I am not done yet. So, don't review atm

Edited 11 months ago by vivekanand1101
11 months ago

rebased

11 months ago

rebased

11 months ago

1 new commit added

  • Show the username only when it's a fork for watch list in index page

Hey good work there , my take would be that watchlist is on the side bellow groups since there is a lot of place you can utilize there!

@ryanlerch you too think that it should be below the groups?

I like, looks nice! :)

10 months ago

rebased

10 months ago

rebased

:thumbsup: for this !

one problem that i see myself is that i have used set() at one point because of that the watch list will be arranged in a different way about every time :stuck_out_tongue: (i will think of something different if anybody doesn't like this)

I don't think that as a big issue , but you can always do sorted(<set>) to achieve order.

10 months ago

rebased

10 months ago

rebased

pingou commented on line 38 of tests/test_pagure_flask_ui_app.py.
10 months ago
(Show)
pep8 will likely ask for a single line here and above the function as well
pingou commented on line 7 of pagure/lib/__init__.py.
10 months ago
(Show)
This could use a test in ``tests/pagure_lib.py``
pingou commented on line 45 of pagure/lib/__init__.py.
10 months ago
(Show)
If you want to sort that list :thumbsup: for me

This is just looking at the code, I'll test it locally for the UI

@ryanlerch if you could also have a look at it from an UI point of view, that would be nice :)

10 months ago

rebased

10 months ago

rebased

:thumbsup: from me from the UI perspective

10 months ago

rebased

10 months ago

rebased

vivekanand1101 commented on line 12 of pagure/lib/__init__.py.
10 months ago
(Show)
I have replaced ```None``` by ```[]``` here. I thought: If this line was ever hit, it would have resulted in a crash because in the template we are using ```| count```.
pingou commented on line 12 of pagure/lib/__init__.py.
10 months ago
(Show)
Jinja is pretty error proof, so I'm not sure it would have mattered but good idea nonetheless :)
pingou commented on line 12 of pagure/lib/__init__.py.
10 months ago
(Show)
> If this line was ever hit, it would have resulted in a crash because in the template we are using | count. This line makes me wonder: are you covering this in your unit-tests? If not they, we should fix it :)

@pingou i used __get_user which raises exception when user is not found. I have changed it to search_user and added the unknown user check in the tests as well. probably, shouldn't have used __get_user. :/

10 months ago

rebased

10 months ago

rebased

10 months ago

rebased

Alright, let's rebase and merge :)

10 months ago

rebased

10 months ago

Pull-Request has been merged by pingou

Changes summary
+41 -0
file changed
+28 -0
file changed
+5 -0
file changed
+30 -1
file changed
+34 -0
file changed