#1158 Show user watch list in the index page of the user
Merged by pingou. Opened by vivekanand1101.
vivekanand1101/pagure watch_page  into  master

Download 1158.patch
no initial comment

rebased

rebased

Fixes #1129

You should use something like unwatch in place of i. i doesn't convey anything.

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 ?

I can see the to-do list of pingou here so, i think it should be fine

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.

rebased

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

rebased

rebased

open to review now.

This is how it looks: http://img.susepaste.org/24954063

1 new commit added

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

ATM: http://img.susepaste.org/52738170

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?

is this one better? http://img.susepaste.org/5851306

I like, looks nice! :)

rebased

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.

rebased

rebased

pep8 will likely ask for a single line here and above the function as well

This could use a test in tests/pagure_lib.py

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 :)

rebased

rebased

this is how it looks: http://img.susepaste.org/69070857

:thumbsup: from me from the UI perspective

rebased

rebased

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.

Jinja is pretty error proof, so I'm not sure it would have mattered but good idea nonetheless :)

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. :/

rebased

rebased

rebased

Alright, let's rebase and merge :)

rebased

Pull-Request has been merged by pingou

Metadata