-
-
Notifications
You must be signed in to change notification settings - Fork 209
Added nest app and slack command for sponsorship #947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cd28fee
e7f54b1
7e81614
faf9968
c448539
00da50e
c6c3ce7
b17a378
a5ac816
482ec15
d8cc69e
f062c55
2949848
763cf8b
956c164
ebcf410
78359aa
c272978
b09daae
19644b7
d8e444f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,9 +1,13 @@ | ||||||||||||||
"""GitHub app common module.""" | ||||||||||||||
|
||||||||||||||
import logging | ||||||||||||||
import os | ||||||||||||||
from datetime import timedelta as td | ||||||||||||||
from urllib.parse import urlparse | ||||||||||||||
|
||||||||||||||
from django.core.exceptions import ValidationError | ||||||||||||||
from django.utils import timezone | ||||||||||||||
from github import Github | ||||||||||||||
from github.GithubException import UnknownObjectException | ||||||||||||||
|
||||||||||||||
from apps.github.models.issue import Issue | ||||||||||||||
|
@@ -16,9 +20,72 @@ | |||||||||||||
from apps.github.models.user import User | ||||||||||||||
from apps.github.utils import check_owasp_site_repository | ||||||||||||||
|
||||||||||||||
FETCH_ISSUE_ERROR = "Failed to fetch issue from GitHub: {error}" | ||||||||||||||
MIN_PARTS_LENGTH = 4 | ||||||||||||||
VALID_PROJECT_ERROR = ( | ||||||||||||||
"Issue does not have a valid project and cannot be considered for sponsorship." | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
logger = logging.getLogger(__name__) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
def sync_issue(issue_link): | ||||||||||||||
"""Sync GitHub issue data.""" | ||||||||||||||
try: | ||||||||||||||
return Issue.objects.get(url=issue_link) | ||||||||||||||
except Issue.DoesNotExist: | ||||||||||||||
pass | ||||||||||||||
|
||||||||||||||
parsed_url = urlparse(issue_link) | ||||||||||||||
path_parts = parsed_url.path.strip("/").split("/") | ||||||||||||||
|
||||||||||||||
Comment on lines
+40
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainValidate URL path before extracting issue components. The code parses the issue link but doesn't validate if it's a proper GitHub URL or if parsed_url = urlparse(issue_link)
path_parts = parsed_url.path.strip("/").split("/")
+
+if len(path_parts) < MIN_PARTS_LENGTH or path_parts[2] != "issues":
+ raise ValidationError(INVALID_ISSUE_LINK_FORMAT) Note: The constant I'll start by running a shell script to search for the definitions of the constants 🏁 Script executed: #!/bin/bash
echo "Searching for MIN_PARTS_LENGTH defined in the repository..."
rg "MIN_PARTS_LENGTH" .
echo "Searching for INVALID_ISSUE_LINK_FORMAT defined in the repository..."
rg "INVALID_ISSUE_LINK_FORMAT" . Length of output: 760 Critical: Validate GitHub Issue URL and Define Missing Constant The updated validation logic correctly checks if the URL path contains enough parts and whether the issue keyword is present. However, while
|
||||||||||||||
github_token = os.getenv("GITHUB_TOKEN") | ||||||||||||||
github_client = Github(github_token) | ||||||||||||||
abhayymishraa marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
issue_number = int(path_parts[3]) | ||||||||||||||
owner = path_parts[0] | ||||||||||||||
repo_name = path_parts[1] | ||||||||||||||
|
||||||||||||||
try: | ||||||||||||||
gh_repo = github_client.get_repo(f"{owner}/{repo_name}") | ||||||||||||||
gh_issue = gh_repo.get_issue(issue_number) | ||||||||||||||
except Exception as error: | ||||||||||||||
raise ValidationError(FETCH_ISSUE_ERROR.format(error=error)) from error | ||||||||||||||
|
||||||||||||||
try: | ||||||||||||||
author = User.objects.get(login=gh_issue.user.login) | ||||||||||||||
except User.DoesNotExist: | ||||||||||||||
author = User.update_data(gh_issue.user) | ||||||||||||||
|
||||||||||||||
try: | ||||||||||||||
repository = Repository.objects.get(node_id=gh_repo.id) | ||||||||||||||
except Repository.DoesNotExist: | ||||||||||||||
try: | ||||||||||||||
repo_owner = User.objects.get(login=gh_repo.owner.login) | ||||||||||||||
except User.DoesNotExist: | ||||||||||||||
repo_owner = User.update_data(gh_repo.owner) | ||||||||||||||
try: | ||||||||||||||
organization = Organization.objects.get(node_id=gh_repo.organization.id) | ||||||||||||||
except Organization.DoesNotExist: | ||||||||||||||
organization = Organization.update_data(gh_repo.organization) | ||||||||||||||
abhayymishraa marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
repository = Repository.update_data( | ||||||||||||||
gh_repository=gh_repo, | ||||||||||||||
commits=gh_repo.get_commits(), | ||||||||||||||
contributors=gh_repo.get_contributors(), | ||||||||||||||
languages=gh_repo.get_languages(), | ||||||||||||||
organization=organization, | ||||||||||||||
user=repo_owner, | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
if not repository.project: | ||||||||||||||
logger.exception(VALID_PROJECT_ERROR) | ||||||||||||||
return None | ||||||||||||||
|
||||||||||||||
Comment on lines
+82
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve project validation error handling. The code checks if the repository has a project, but only logs an error and returns None rather than raising the ValidationError with the message defined in the if not repository.project:
- logger.exception(VALID_PROJECT_ERROR)
- return None
+ logger.error(VALID_PROJECT_ERROR)
+ raise ValidationError(VALID_PROJECT_ERROR) 📝 Committable suggestion
Suggested change
|
||||||||||||||
return Issue.update_data(gh_issue, author=author, repository=repository) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
def sync_repository(gh_repository, organization=None, user=None): | ||||||||||||||
"""Sync GitHub repository data. | ||||||||||||||
|
||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
from django.contrib import admin | ||
|
||
from apps.nest.models.sponsorship import Sponsorship | ||
|
||
|
||
class SponsorshipAdmin(admin.ModelAdmin): | ||
list_display = ("issue", "amount", "slack_user_id") | ||
search_fields = ("issue__title", "slack_user_id") | ||
|
||
|
||
admin.site.register(Sponsorship, SponsorshipAdmin) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
from django.apps import AppConfig | ||
|
||
|
||
class NestConfig(AppConfig): | ||
name = "apps.nest" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# Generated by Django 5.1.5 on 2025-03-16 07:43 | ||
|
||
import django.db.models.deletion | ||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
initial = True | ||
|
||
dependencies = [ | ||
("github", "0016_user_is_bot"), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name="Sponsorship", | ||
fields=[ | ||
( | ||
"id", | ||
models.BigAutoField( | ||
auto_created=True, | ||
primary_key=True, | ||
serialize=False, | ||
verbose_name="ID", | ||
), | ||
), | ||
("nest_created_at", models.DateTimeField(auto_now_add=True)), | ||
("nest_updated_at", models.DateTimeField(auto_now=True)), | ||
( | ||
"currency", | ||
models.CharField( | ||
choices=[("EUR", "Euro"), ("USD", "US Dollar")], | ||
default="USD", | ||
max_length=3, | ||
), | ||
), | ||
("deadline_at", models.DateTimeField(blank=True, null=True)), | ||
("amount", models.FloatField()), | ||
("slack_user_id", models.CharField(max_length=100)), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainValidate the slack_user_id field. Consider adding validation for the slack_user_id field format to ensure it matches Slack's user ID pattern. 🏁 Script executed: #!/bin/bash
# Check if the Sponsorship model has validation for slack_user_id in the model definition
# Find the Sponsorship model definition
fd -e py -t f "sponsorship.py" | xargs grep -A 20 "class Sponsorship" Length of output: 686 Action Required: Implement Slack User ID Validation The Sponsorship model currently defines the |
||
( | ||
"issue", | ||
models.ForeignKey( | ||
on_delete=django.db.models.deletion.CASCADE, | ||
related_name="sponsorships", | ||
to="github.issue", | ||
), | ||
), | ||
], | ||
options={ | ||
"verbose_name_plural": "Sponsorships", | ||
"db_table": "nest_sponsorships", | ||
}, | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
"""Nest app sponsorship model.""" | ||
|
||
from django.core.exceptions import ValidationError | ||
from django.db import models | ||
from django.utils import timezone | ||
|
||
from apps.common.models import BulkSaveModel, TimestampedModel | ||
from apps.github.models.issue import Issue | ||
|
||
DEADLINE_FUTURE_ERROR = "Deadline must be in the future." | ||
ISSUE_LINK_ERROR = "Issue link must belong to an OWASP repository." | ||
PRICE_POSITIVE_ERROR = "Price must be a positive value." | ||
|
||
|
||
class Sponsorship(BulkSaveModel, TimestampedModel): | ||
"""Sponsorship model.""" | ||
|
||
class CurrencyType(models.TextChoices): | ||
"""Currency type choices.""" | ||
|
||
EUR = "EUR", "Euro" | ||
USD = "USD", "US Dollar" | ||
|
||
class Meta: | ||
db_table = "nest_sponsorships" | ||
verbose_name_plural = "Sponsorships" | ||
|
||
currency = models.CharField( | ||
max_length=3, choices=CurrencyType.choices, default=CurrencyType.USD | ||
) | ||
deadline_at = models.DateTimeField(null=True, blank=True) | ||
amount = models.FloatField() | ||
slack_user_id = models.CharField(max_length=100) | ||
|
||
issue = models.ForeignKey( | ||
Issue, | ||
on_delete=models.CASCADE, | ||
related_name="sponsorships", | ||
) | ||
|
||
def __str__(self): | ||
"""Sponsorship human readable representation.""" | ||
return f"Sponsorship for {self.issue.title} by {self.slack_user_id}" | ||
|
||
def clean(self): | ||
"""Validate model data.""" | ||
super().clean() | ||
|
||
# Validate amount is positive | ||
if self.amount <= 0: | ||
raise ValidationError(PRICE_POSITIVE_ERROR) | ||
|
||
# Validate deadline is in the future | ||
if self.deadline_at and self.deadline_at < timezone.now(): | ||
raise ValidationError(DEADLINE_FUTURE_ERROR) | ||
|
||
# Validate GitHub issue link belongs to OWASP | ||
if not self.issue.url.startswith("https://github.com/OWASP"): | ||
raise ValidationError(ISSUE_LINK_ERROR) | ||
|
||
def save(self, *args, **kwargs): | ||
"""Override save to run full validation.""" | ||
self.full_clean() | ||
super().save(*args, **kwargs) | ||
|
||
@staticmethod | ||
def update_data(sponsorship, **kwargs): | ||
"""Update sponsorship data with the provided fields.""" | ||
fields_to_update = ["amount", "deadline_at", "slack_user_id"] | ||
for field in fields_to_update: | ||
if field in kwargs: | ||
setattr(sponsorship, field, kwargs[field]) | ||
sponsorship.save() | ||
return sponsorship |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding a validation constant for malformed issue links.
While these constants help standardize error messages, there's no definition for something like
INVALID_ISSUE_LINK_FORMAT
if you plan to validate the link structure. Additionally, ifMIN_PARTS_LENGTH
remains unused, consider removing or referencing it when validatingpath_parts
.