user's permissions on Issues/PRs

[marusinm]

Updated by @lachmanfrantisek :

We have a lot of methods related to the permissions (e.g. who can merge/close,..)

It's not consistent, let's clean that!

Feel free to solve this in smaller parts. Just write on what you are going to work...

AC:

• Make sure, that we have all of the provided methods in the abstract.py
  (raising the NotImplementedError)
• Make sure, that we have all of these in all the implementations:
  • GitLab
  • GitHub
  • Pagure
• Deprecate the project methods related to specific issue/pull-request.
• Move helping methods to the base classes.
  • The only exception is a more efficient solution in the code of the specific implementation.
  • Example:
    https://github.com/packit-service/ogr/blob/89435655150b8dd79ef93209c3463585d0715e0d/ogr/services/github/project.py#L219-L220

The progress is tracked in the following tables. (Any update in the comments is appreciated.)

Project	who_can_close_issue() -> Set[str]	who_can_merge_pr() -> Set[str]	can_close_issue(username: str) -> bool	can_merge_pr(username: str) -> bool
abstract	👍	👍	❗ deprecate the Issue argument
base	nothing	nothing	implement here	implement here
Github	?	?	?	?
GitLab	?	?	?	?
Pagure	?	?	?	?

Issue	who_can_close() -> Set[str]	can_close(username: str) -> bool
abstract	implement here	👍
base	nothing	👍
Github	?	nothing
GitLab	?	nothing
Pagure	?	nothing

PullRequest	who_can_close() -> Set[str]	who_can_merge() -> Set[str]	can_close(username: str) -> bool	can_merge(username: str) -> bool
abstract	?	?	?	?
base	nothing	nothing	implement here	implement here
Github	?	?	?	?
GitLab	?	?	?	?
Pagure	?	?	?	?

[lachmanfrantisek]

I've updated the description. This issue is now about cleaning the methods and add the missing parts. Please, check if I don't forget anything.

[pranav33317]

Yes ma'am , I am working on it , will draft a PR ASAP . Had a question about the unimplemented functions , a lot of the functions are missing return statements and thus giving error . Should I just return default value or raise some error ?
Thanks

[lbarcziova]

@pranav33317 hello and thanks!

a lot of the functions are missing return statements and thus giving error .

Could you please provide some examples?

[pranav33317]

@indirect(GithubIssue.get)
def get_issue(self, issue_id: int) -> Issue:
    pass

@indirect(GithubIssue.create)
def create_issue(
    self,
    title: str,
    body: str,
    private: Optional[bool] = None,
    labels: Optional[list[str]] = None,
    assignees: Optional[list[str]] = None,
) -> Issue:
    pass , @indirect(GithubPullRequest.get_list)
def get_pr_list(self, status: PRStatus = PRStatus.open) -> list[PullRequest]:
    pass
 def get_issue_list(
          self,
          status: IssueStatus = IssueStatus.open,
          author: Optional[str] = None,
          assignee: Optional[str] = None,
          labels: Optional[list[str]] = None,
      ) -> list[Issue]:
            pass

@indirect(GithubPullRequest.get)
def get_pr(self, pr_id: int) -> PullRequest:
    pass , def create_pr(
    self,
    title: str,
    body: str,
    target_branch: str,
    source_branch: str,
    fork_username: Optional[str] = None,
) -> PullRequest:
    pass, def set_commit_status(
    self,
    commit: str,
    state: Union[CommitStatus, str],
    target_url: str,
    description: str,
    context: str,
    trim: bool = False,
):
    pass

@indirect(GithubCommitFlag.get)
def get_commit_statuses(self, commit: str) -> list[CommitFlag]:
    pass

@indirect(GithubCheckRun.get)
def get_check_run(
    self,
    check_run_id: Optional[int] = None,
    commit_sha: Optional[str] = None,
) -> Optional["GithubCheckRun"]:
    pass

@indirect(GithubCheckRun.create)
def create_check_run(
    self,
    name: str,
    commit_sha: str,
    url: Optional[str] = None,
    external_id: Optional[str] = None,
    status: GithubCheckRunStatus = GithubCheckRunStatus.queued,
    started_at: Optional[datetime.datetime] = None,
    conclusion: Optional[GithubCheckRunResult] = None,
    completed_at: Optional[datetime.datetime] = None,
    output: Optional[GithubCheckRunOutput] = None,
    actions: Optional[list[dict[str, str]]] = None,
) -> "GithubCheckRun":
    pass

@indirect(GithubCheckRun.get_list)
def get_check_runs(
    self,
    commit_sha: str,
    name: Optional[str] = None,
    status: Optional[GithubCheckRunStatus] = None,
) -> list["GithubCheckRun"]:
    pass

, these are some of the examples of unimplemented functions missing return statements , they are mostly in the following files : ogr/services/github/project.py and ogr/services/gitlab/project.py

[lbarcziova]

and where do you see these causing errors? From what you posted I would say the methods are implemented, just not directly, but using the @indirect decorator, with methods on the particular objects.

[pranav33317]

All of them have empty bodies and require a return statement so they are returning error :

[pranav33317]

Hi , I'm sorry I commented on the wrong issue . This is related to #696 , I will repost it there.
I'm working on this issue parallely as well.

[Gauravpadam]

The description is helpful. I'm looking into this @lachmanfrantisek

[Gauravpadam]

If I understand this correctly:

• abstract.py contains all the abstractions for the class methods. We are supposed to have all the mentioned methods as abstract methods first.
• Once that is done, We can actually start implementing these methods into their respective SubClasses in github.py, gitlab.py and pagure.py.
• We will write some unit tests for these changes.

Am I thinking in the right direction @lachmanfrantisek @lbarcziova?

If yes, I'll draft a PR for this ticket

[lbarcziova]

hi @Gauravpadam, yes what you wrote is mostly correct. The implementation itself should be in subclasses, where it is possible (supported by forge) and abstract.py should just contain the signatures. And the base class can serve for common logic. Also, sticking to the description and link can help; only the tables might be a bit outdated. But let us know if anything else is unclear.

[pranav33317]

Ma'am I was waiting for a response , could you please clear up my doubt , I was working on this

[lbarcziova]

@pranav33317 sorry, I got confused since you were mentioning #696 in the last comment. So what's the current status for this issue, are you blocked on anything at the moment? (I understood #100 (comment) is related to #696)

[pranav33317]

Yes ma'am still working on both . Could you please answer my query in #696 ?