pagure

Clone

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

No commits found

no initial comment

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

1 new commit added

  • Fix duplicate lines
7 years ago

1 new commit added

  • Fix pep8 conventions
7 years ago

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
7 years ago

1 new commit added

  • Add check for api endpoint for private repo
7 years ago

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
7 years ago

You're passing the function as argument?

1 new commit added

  • Add a different method to fetch project
7 years ago

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
7 years ago

2 new commits added

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

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
7 years ago

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
7 years ago

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 7 years ago by vivekanand1101

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
7 years ago

1 new commit added

  • Fix docstring for get_authorized_project
7 years ago

1 new commit added

  • Remove the card-header check
7 years ago

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
7 years ago

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

Pull-Request has been merged by pingou
7 years ago
Metadata
None
No Tags
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.