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

Proposed a year ago by vivekanand1101
Modified a year ago
From forks/vivekanand1101/pagure watch_page  into pagure master
file changed

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

                  return True

  

      return False

+ 

+ 

+ def user_watch_list(session, user):

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

+ 

+     user_obj = search_user(session, username=user)

+     if not user_obj:

+         return []

+ 

+     unwatched = session.query(

+         model.Watcher

+     ).filter(

+         model.Watcher.user_id == user_obj.id

+     ).filter(

+         model.Watcher.watch == False

+     )

+ 

+     unwatched_list = []

+     if unwatched:

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

+ 

+     watched = session.query(

+         model.Watcher

+     ).filter(

+         model.Watcher.user_id == user_obj.id

+     ).filter(

+         model.Watcher.watch == True

+     )

+ 

+     watched_list = []

+     if watched:

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

+ 

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

+     watch = set(watched_list + user_projects)

+ 

+     for project in user_projects:

+         if project in unwatched_list:

+             watch.remove(project)

+ 

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

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

              </div>

              {% endfor %}

          </div>

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

+             <div class="card-header">

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

+             </div>

+             {% for repo in watch_list %}

+               <div class="list-group">

+                 {% if repo.is_fork %}

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

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

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

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

+                     </div>

+                   </a>

+                 {% else %}

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

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

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

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

+                     </div>

+                   </a>

+                 {% endif %}

+               </div>

+             {% else %}

+             <div class="card-block">

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

+             </div>

+             {% endfor %}

+         </div>

        </div>

      </div>

    </div>
file changed

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

          fork=True,

          count=True)

  

+     watch_list = pagure.lib.user_watch_list(

+         SESSION,

+         user=flask.g.fas_user.username)

+ 

      return flask.render_template(

          'index_auth.html',

          username=flask.g.fas_user.username,

          user=user,

          forks=forks,

          repos=repos,

+         watch_list=watch_list,

          repopage=repopage,

          forkpage=forkpage,

          repos_length=repos_length,
file changed

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

              self.assertEqual(

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

              self.assertEqual(

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

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

+ 

+     def test_watch_list(self):

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

+ 

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

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

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

+             self.assertIn(

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

+                 output.data)

+             self.assertIn(

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

+                 output.data)

+             self.assertIn(

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

+                 output.data)

+ 

+             tests.create_projects(self.session)

+ 

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

+             self.assertIn(

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

+                 output.data)

+             self.assertIn(

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

+                 output.data)

+             self.assertIn(

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

+                 output.data)

  

      def test_view_users(self):

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

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

          self.assertFalse(watch)

  

  

+     def test_user_watch_list(self):

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

+ 

+         tests.create_projects(self.session)

+ 

+         # He should be watching

+         user = tests.FakeUser()

+         user.username = 'pingou'

+         watch_list_objs = pagure.lib.user_watch_list(

+             session=self.session,

+             user='pingou',

+         )

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

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

+ 

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

+         user.username = 'vivek'

+         watch_list_objs = pagure.lib.user_watch_list(

+             session=self.session,

+             user='vivek',

+         )

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

+         self.assertEqual(watch_list, [])

+ 

+         # He shouldn't be watching anything

+         user.username = 'foo'

+         watch_list_objs = pagure.lib.user_watch_list(

+             session=self.session,

+             user='foo',

+         )

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

+         self.assertEqual(watch_list, [])

+ 

+ 

  if __name__ == '__main__':

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

      unittest.TextTestRunner(verbosity=2).run(SUITE)
a year ago

rebased

a year ago

rebased

farhaan commented on line 20 of pagure/lib/__init__.py.
a year ago
(Hide)
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.
a year ago
(Hide)
here!
farhaan commented on line 7 of pagure/ui/app.py.
a year ago
(Hide)
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.
a year ago
(Hide)
I can see the to-do list of pingou [here](https://pagure.io/user/pingou/requests) so, i think it should be fine
a year 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.

a year ago

rebased

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

Edited a year ago by vivekanand1101
a year ago

rebased

a year ago

rebased

open to review now.

a year 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! :)

a year ago

rebased

a year 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.

a year ago

rebased

a year ago

rebased

pingou commented on line 38 of tests/test_pagure_flask_ui_app.py.
a year ago
(Hide)
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.
a year ago
(Hide)
This could use a test in ``tests/pagure_lib.py``
pingou commented on line 45 of pagure/lib/__init__.py.
a year ago
(Hide)
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 :)

a year ago

rebased

a year ago

rebased

:thumbsup: from me from the UI perspective

a year ago

rebased

a year ago

rebased

vivekanand1101 commented on line 12 of pagure/lib/__init__.py.
a year ago
(Hide)
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.
a year ago
(Hide)
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.
a year ago
(Hide)
> 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. :/

a year ago

rebased

a year ago

rebased

a year ago

rebased

Alright, let's rebase and merge :)

a year ago

rebased

a year 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