pagure

#1103 Add Test case for private-repo
Merged by pingou. Opened by farhaan.
farhaan/pagure private-repo  into  private_repo

Download 1103.patch
no initial comment

This one is a duplicate, so maybe we want to remove the dups :)

1 new commit added

  • Fix duplicate lines

1 new commit added

  • Fix pep8 conventions

3 new commits added

  • Add check for repo admin when repo is private
  • Add test to check issues and pull-request
  • Add test for issues to private repo and changes to fix those test

1 new commit added

  • Add check for api endpoint for private repo

Is this line pep8 valid?

Also, is this a 403 or a 401?

When I see the amount of duplicated code I wonder if we shouldn't refactor a little bit (but maybe in another PR)

There are all together 3 projects, 2 for this user and the last one is?

We ran tests.create_projects(self.session) then create 2 projects?

Maybe we should check that this is 0 before we add pingou to this project?

Duplicated paragraph?

pep8 :

pep8 :)

and here as well :)

3 projects are publicly viewed two are 'pingou' 's project which 'tests.create_projects(self.session)' creates and one is 'foo' 's public repository . Here 'foo' is viewing his repos so it has one public and one private repo.

One public project and the other private project with a different user.

I'm not sure 403 or 401 are the best ways to go here... you will likely not even want to leak the existence of private repos, so giving the same 404 as a project that just doesn't exist would likely be better.

Also, would it be an idea to instead do the permission check in pagure.lib.get_project? That way it automatically covers every place the repo is used.

And how many projects are created by tests.create_projects(self.session) ?

And you probably want the "if repo is None" above this..

:thumbsup: indeed

4 new commits added

  • Fix pep8 issues
  • Fix parameter placement for positional parameter
  • Add is_repo_admin parameter to get_project
  • Fix pep 8 issues and remove duplication

You're passing the function as argument?

1 new commit added

  • Add a different method to fetch project

21 new commits added

  • Add a different method to fetch project
  • Fix pep8 issues
  • Fix parameter placement for positional parameter
  • Add is_repo_admin parameter to get_project
  • Fix pep 8 issues and remove duplication
  • Add check for api endpoint for private repo
  • Add check for repo admin when repo is private
  • Add test to check issues and pull-request
  • Add test for issues to private repo and changes to fix those test
  • Fix pep8 conventions
  • Fix duplicate lines
  • Add test for adding user to the private repo
  • Fix pull-requests UI for private repo
  • Fix test for pull-request for private repo
  • Fix GIT_FOLDER in test for private repo
  • Add change for PR
  • Add test to check private checkbox UI
  • Fix test private repo UI
  • Add Test for UI and Pull request to Private Repo
  • Check if the user authenticated is the one for whom we're viewing the repos
  • Add test for private repo

2 new commits added

  • Add test for api end point to get projects
  • Fix api point to fetch tags

There are 4 projects total in the DB, here we see the 3 that are public and below we see 2, could we add a test to see either the remaining 2 and all the 4 at once?

80 chars long?

Technically we don't check if they are accessible but we check that they are not accessible :)

But I'm nit picking here :)

We didn't check that for a non-authenticated user

1 new commit added

  • Add test for pull request api and condition for pull request api(private

9 new commits added

  • Test for private repo view issue comment api endpoint
  • Add test for adding comment to issue api endpoint
  • Add test for changing status through api point in private repo
  • Add tests to test issues api for private repo
  • Fix issue api to support private projects
  • Add test for Pr merge and close in private repo
  • Add test to flag PR in private repo
  • Add test private repo api endpoit for adding comment to PR
  • Add test a single PR retrival

The doc string here is misleading. It should better be something like Returns the repo object in cases when the repo is not private and if the logged in user is authorized to view the repo, else None
Edited by vivekanand1101

This import is called here: https://pagure.io/fork/farhaan/pagure/blob/private-repo/f/pagure/ui/fork.py#_999

Thanks for pointing that out :smile:

Extra space here and here

while listing "All projects" in the index page, should we include or not include, the count of private repos while telling the total number of projects?

while listing "All projects" in the index page, should we include or not include, the count of private repos while telling the total number of projects?

We should't include count of private repos!

2 new commits added

  • Fix test for repo and fork ui
  • Fix missing import and pep 8

1 new commit added

  • Fix docstring for get_authorized_project

1 new commit added

  • Remove the card-header check

Btw, to help being compatible w/ py3, could you use output.get_data(as_text=True) everywhere you use output.data, just doing a global search and replace should do :)

37 new commits added

  • Fix test failling for card-header count and py3 compatibility
  • Fix docstring for get_authorized_project
  • Fix test for repo and fork ui
  • Fix missing import and pep 8
  • Test for private repo view issue comment api endpoint
  • Add test for adding comment to issue api endpoint
  • Add test for changing status through api point in private repo
  • Add tests to test issues api for private repo
  • Fix issue api to support private projects
  • Add test for Pr merge and close in private repo
  • Add test to flag PR in private repo
  • Add test private repo api endpoit for adding comment to PR
  • Add test a single PR retrival
  • Add test for pull request api and condition for pull request api(private
  • Add test for api end point to get projects
  • Fix api point to fetch tags
  • Add a different method to fetch project
  • Fix pep8 issues
  • Fix parameter placement for positional parameter
  • Add is_repo_admin parameter to get_project
  • Fix pep 8 issues and remove duplication
  • Add check for api endpoint for private repo
  • Add check for repo admin when repo is private
  • Add test to check issues and pull-request
  • Add test for issues to private repo and changes to fix those test
  • Fix pep8 conventions
  • Fix duplicate lines
  • Add test for adding user to the private repo
  • Fix pull-requests UI for private repo
  • Fix test for pull-request for private repo
  • Fix GIT_FOLDER in test for private repo
  • Add change for PR
  • Add test to check private checkbox UI
  • Fix test private repo UI
  • Add Test for UI and Pull request to Private Repo
  • Check if the user authenticated is the one for whom we're viewing the repos
  • Add test for private repo

Alright, looks good to me, let's merge :)

Pull-Request has been merged by pingou
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.