Skip to content

feat: refactor payload and headers into pure JSON #44

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

Merged
merged 8 commits into from
Jun 13, 2025
Merged

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 13, 2025

This PR refactors the webhook payload and headers handling to use pure JSON structures with string keys, removing the symbolization options that were causing complexity and inconsistency.

Changes Made

Configuration Changes

  • Removed symbolize_payload and symbolize_headers configuration options from default config
  • Removed these options from environment variable mappings and config validation schema
  • Kept normalize_headers option for consistent header formatting (lowercasing and trimming)

API Processing Updates

  • Updated API to always pass payload and headers with string keys
  • Removed symbolization logic from request processing pipeline
  • Maintained header normalization capabilities when enabled

Helper Method Updates

  • Updated parse_payload method to default to symbolize: false
  • Kept backward compatibility parameter but changed default behavior
  • Ensured JSON parsing always returns pure hash structures

Documentation Updates

  • Updated docs/handler_plugins.md to show string key examples instead of symbol keys
  • Updated method signatures and examples to reflect new pure JSON approach
  • Clarified that config parameter remains symbolized as it's controlled by the framework

Test Updates

  • Updated all unit tests to expect string keys instead of symbols
  • Fixed integration tests to validate new behavior
  • Updated acceptance test handlers to work with string-based payload access
  • Renamed header symbolization integration test to reflect new focus on normalization

Examples

Before (symbolized):

# Accessing payload and headers required symbol keys
puts payload[:some_cool_value]
puts headers[:x_custom_header]

After (pure JSON):

# Clean JSON access with string keys
puts payload["some_cool_value"]
puts headers["x-custom-header"]

Benefits

  • Payload and headers are now pure JSON structures that can be easily serialized
  • Eliminates confusion about when keys are symbols vs strings
  • Simplifies the codebase by removing symbolization complexity
  • Maintains all normalization capabilities for headers
  • Config parameter remains symbolized as it's framework-controlled

All tests pass including unit, integration, and acceptance tests. The changes maintain backward compatibility while providing a cleaner, more predictable API.

Fixes #43.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] feat: refactor payload and headers into pure JSON feat: refactor payload and headers into pure JSON Jun 13, 2025
@Copilot Copilot AI requested a review from GrantBirki June 13, 2025 06:00
Copilot finished work on behalf of GrantBirki June 13, 2025 06:00
@GrantBirki GrantBirki marked this pull request as ready for review June 13, 2025 06:02
@Copilot Copilot AI review requested due to automatic review settings June 13, 2025 06:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors payload and headers handling to use pure JSON with string keys, removing symbolization options and simplifying normalization.

  • Removed symbolize_payload and symbolize_headers options from configuration and validation
  • Updated parsing logic and API to default to string-key hashes for payloads and headers
  • Revised tests and documentation to align with the new JSON-centric approach

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/unit/lib/hooks/core/config_validator_spec.rb Removed tests related to symbolization options
spec/unit/lib/hooks/core/config_loader_spec.rb Updated loader specs to focus on normalize_headers only
spec/unit/lib/hooks/app/helpers_spec.rb Changed expectations to string-key hashes
spec/integration/header_symbolization_spec.rb Adjusted integration tests for header normalization
spec/acceptance/plugins/handlers/team1_handler.rb Updated handler example to use string-key access
lib/hooks/core/config_validator.rb Removed symbolization schema entries
lib/hooks/core/config_loader.rb Removed default symbolization configs
lib/hooks/app/helpers.rb Updated parse_payload signature and docs
lib/hooks/app/api.rb Passed symbolize: false and adjusted header logic
docs/handler_plugins.md Revised docs to show string-key JSON examples
.bundle/config Updated Bundler paths and deployment settings
Comments suppressed due to low confidence (3)

spec/unit/lib/hooks/core/config_loader_spec.rb:370

  • [nitpick] The test description is vague; consider renaming it to "allows disabling normalize_headers via environment variable" for clarity.
it "allows environment variable setup" do

spec/integration/header_symbolization_spec.rb:1

  • [nitpick] The file name still references symbolization but tests normalization only. Consider renaming it to header_normalization_spec.rb to match its content.
spec/integration/header_symbolization_spec.rb

.bundle/config:3

  • Committing environment-specific absolute paths can cause unwanted diffs; revert to a relative project path or exclude .bundle/config from version control.
BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle"

@GrantBirki GrantBirki merged commit 52fb87f into main Jun 13, 2025
21 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-43 branch June 13, 2025 06:12
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.

feat: refactor payload and headers into pure JSON
2 participants