sagemathgh-36213: Fix sync labels issues for step 2 going live

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

This is a follow up PR of sagemath#35172.
It addresses the issues observed in the second step of going live
(sagemath#35927 (comment))
of the label sync bot, which had to be disabled after a day
(sagemath#35927 (comment)).
As observed in
sagemath#36177 (comment),
this was caused by a bug concerning the `reviewDecision` in case the
reviewer does not have appropriate access rights to the repository. In
this case `gh pr view` returns None as `reviewDecision` which has been
mapped to `COMMENTED` by caching reasons. The bug is that comparisons
with `reviewDecision` have not been adapted to the cached value.

Furthermore, the log-file sequence displayed in
sagemath#36177 (comment)
shows another unexpected behavior:

```
> INFO:root:Proper reviews after 2023-09-02T07:55:03Z for pull request
sagemath#36177: [{'id': 'PRR_kwDOI5-Tx85f2_5f', 'author': {'login': 'dcoudert'},
'authorAssociation': 'CONTRIBUTOR', 'body': 'LGTM.', 'submittedAt':
'2023-09-02T13:23:57Z', 'includesCreatedEdit': False, 'reactionGroups':
[], 'state': 'APPROVED', 'commit': {'oid':
'626b764283b5005ab3a64661f7cc74efcd31adb9'}}, {'id':
'PRR_kwDOI5-Tx85f3sRb', 'author': {'login': 'tobiasdiez'},
'authorAssociation': 'CONTRIBUTOR', 'body': "Let's see if its works when
admins approve a PR", 'submittedAt': '2023-09-03T06:08:30Z',
'includesCreatedEdit': False, 'reactionGroups': [], 'state': 'APPROVED',
'commit': {'oid': '626b764283b5005ab3a64661f7cc74efcd31adb9'}}]
```

Here I had expected `'authorAssociation': 'MEMBER'` instead of
`'authorAssociation': 'CONTRIBUTOR'` as it is shown when I do the query
on my local command-line:


```
gh pr view sagemath#36177 --json reviews
{
  "reviews": [
    {
      "author": {
        "login": "dcoudert"
      },
      "authorAssociation": "MEMBER",
      "body": "LGTM.",
      "submittedAt": "2023-09-02T13:23:57Z",
      "includesCreatedEdit": false,
      "reactionGroups": [],
      "state": "APPROVED"
    },
    {
      "author": {
        "login": "tobiasdiez"
      },
      "authorAssociation": "MEMBER",
      "body": "Let's see if its works when admins approve a PR",
      "submittedAt": "2023-09-03T06:08:30Z",
      "includesCreatedEdit": false,
      "reactionGroups": [],
      "state": "APPROVED"
    }
  ]
}
```

As described in flutter/flutter#101012 this is
caused since the github bot is not a member of the sagemath organisation
and most of our members (me too) have set the private role which makes
their membership just visible to other members.

This is relevant in the `approve_allowed` method:

```
    def approve_allowed(self):
        r"""
        Return if the actor has permission to approve this PR.
        """
        revs = self.get_reviews(complete=True)
        if not any(rev['authorAssociation'] in ('MEMBER', 'OWNER') for
rev in revs):
            info('PR %s can\'t be approved because of missing member
review' % (self._issue))
            return False
        ....
```

where the bot checks if it should set the `approved` state by itself.
This did also fail in the example (from the [log-file](https://github.co
m/sagemath/sage/actions/runs/6060223064/job/16444158489)):


```
INFO:root:PR pull request sagemath#36177 doesn't have positve review (by
decision)
INFO:root:PR pull request sagemath#36177 can't be approved because of missing
member review
```

I think we can eliminate the check for a member review entirely as it is
an unnecessary restriction. The check is only relevant if an `s:
positive review` label is added, which can only be done by Triage
members who know what they are doing. Even if we just added
`CONTRIBUTOR` to the list, it would still be restrictive, as it would
not be possible to set `s: positive review` when there is no review
(this would be annoying for trivial PR that just has some PEP8 fixes).


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36213
Reported by: Sebastian Oehms
Reviewer(s): Tobias Diez

.github/sync_labels.py

@@ -56,7 +56,7 @@ class ReviewDecision(Enum):
     """
     changes_requested = 'CHANGES_REQUESTED'
     approved = 'APPROVED'
-    unclear = 'COMMENTED'
+    unclear = 'UNCLEAR'
 
 class Priority(Enum):
     r"""
@@ -319,12 +319,15 @@ def get_review_decision(self):
             return None
 
         if self._review_decision is not None:
+            if self._review_decision == ReviewDecision.unclear:
+                return None
             return self._review_decision
 
         data = self.view('reviewDecision')
         if data:
             self._review_decision = ReviewDecision(data)
         else:
+            # To separate a not supplied value from not cached (see https://github.com/sagemath/sage/pull/36177#issuecomment-1704022893 ff)
             self._review_decision = ReviewDecision.unclear
         info('Review decision for %s: %s' % (self._issue, self._review_decision.value))
         return self._review_decision
@@ -349,9 +352,9 @@ def get_reviews(self, complete=False):
             self.get_commits()
 
         date = self._commit_date
-        no_rev = ReviewDecision.unclear.value
+        unproper_rev = RevState.commented.value
         new_revs = [rev for rev in self._reviews if rev['submittedAt'] > date]
-        proper_new_revs = [rev for rev in new_revs if rev['state'] != no_rev]
+        proper_new_revs = [rev for rev in new_revs if rev['state'] != unproper_rev]
         info('Proper reviews after %s for %s: %s' % (date, self._issue, proper_new_revs))
         return proper_new_revs
 
@@ -463,11 +466,6 @@ def approve_allowed(self):
         r"""
         Return if the actor has permission to approve this PR.
         """
-        revs = self.get_reviews(complete=True)
-        if not any(rev['authorAssociation'] in ('MEMBER', 'OWNER') for rev in revs):
-            info('PR %s can\'t be approved because of missing member review' % (self._issue))
-            return False
-
         revs = self.get_reviews()
         revs = [rev for rev in revs if rev['author']['login'] != self._actor]
         ch_req = ReviewDecision.changes_requested