Skip to content

Conversation

@mmoreiradj
Copy link

Add repository, sender, and installation fields to IssueCommentWebhookEventPayload
struct to match the actual GitHub webhook payload structure.

  • Add repository field (Repository type)
  • Add sender field (Author type)
  • Add installation field (Option type)
  • Add InstallationLite struct for lightweight installation info
  • Add comprehensive test with real webhook payload
  • Update imports to include new required types

This allows consumers to:

  • Identify which repository the comment was made in
  • Identify who triggered the event
  • Access installation information for GitHub Apps

Closes #810

Signed-off-by: Martin Moreira de Jesus martin.moreira-de-jesus@protonmail.com

Add repository, sender, and installation fields to IssueCommentWebhookEventPayload
struct to match the actual GitHub webhook payload structure.

- Add repository field (Repository type)
- Add sender field (Author type)
- Add installation field (Option<InstallationLite> type)
- Add InstallationLite struct for lightweight installation info
- Add comprehensive test with real webhook payload
- Update imports to include new required types

This allows consumers to:
- Identify which repository the comment was made in
- Identify who triggered the event
- Access installation information for GitHub Apps

Closes XAMPPRocky#810

Signed-off-by: Martin Moreira de Jesus <martin.moreira-de-jesus@protonmail.com>
@mmoreiradj
Copy link
Author

Hey, I need this because I'm integrating a rust operator that allows users to create ephemeral environments on ArgoCD
https://github.com/mmoreiradj/sandcastle

Signed-off-by: Martin Moreira de Jesus <martin.moreira-de-jesus@protonmail.com>
Signed-off-by: Martin Moreira de Jesus <martin.moreira-de-jesus@protonmail.com>
@mmoreiradj
Copy link
Author

mmoreiradj commented Oct 8, 2025

By fixing the tests, I just found out that you're supposed to use this struct that way:

let event_header = headers.event.into_inner();
let event_header_str = serde_json::to_string(&event_header).unwrap();
let webhook_event = WebhookEvent::try_from_header_and_body(&event_header_str, &payload.to_string()).unwrap();
let event_payload = event_header.parse_specific_payload(payload).unwrap();
match event_payload {
    WebhookEventPayload::IssueComment(payload) => {
        info!("repository: {:?}", webhook_event.repository);
        info!("sender: {:?}", webhook_event.sender);
        info!("installation: {:?}", webhook_event.installation);
    }
    _ => {
        info!("received unhandled event {:?}", event_payload);
    }
}

Is this really needed or should we keep things the way they are ? @XAMPPRocky


#[test]
fn should_deserialize_with_correct_payload() {
let json = include_str!("../../../../tests/resources/issue_comment_webhook_webhook.json");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be "resources/issue_comment_webhook_webhook.json", no?

@XAMPPRocky
Copy link
Owner

@mmoreiradj Can you clarify? I'm not sure what part you asking about.

@mmoreiradj
Copy link
Author

@mmoreiradj Can you clarify? I'm not sure what part you asking about.

What I mean is that this PR adds the fields to the webhook payload (repository, installation and sender), but they are already present on webhook event which you can build from the webhook payload + event header.

So this modification really necessary ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IssueCommentWebhookEventPayload Missing Fields

3 participants