Skip to content

Conversation

@KshitijaChoudhari
Copy link
Collaborator

  • Implement complete CRUD operations for notification configurations
  • Add models for notification configuration with validation
  • Support workspace and team notification configurations
  • Add comprehensive unit tests with 31 test cases
  • Include example usage with sanitized webhook URLs
  • Based on Go TFE notification_configuration implementation

Description

Testing plan

External links

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

Rollback Plan

Changes to Security Controls

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

If you have any questions, please contact your direct supervisor, GRC (#team-grc), or the PCI working group (#proj-pci-reboot). You can also find more information at PCI Compliance.

@KshitijaChoudhari KshitijaChoudhari requested a review from a team as a code owner October 9, 2025 10:25
- Support workspace and team notification configurations
- Add comprehensive unit tests with 31 test cases
- Include example usage with sanitized webhook URLs
- Based on Go TFE notification_configuration implementation
- Add explicit type annotations for all class attributes
- Update to modern Python type syntax (list instead of List, | None instead of Optional)
- Fix Collection[str] issues by adding explicit type annotations
- All 31 unit tests still passing
- Ruff compliance maintained
- Only 3 MyPy unreachable warnings remain (false positives)
- Clean up documentation to remove Go TFE implementation references
- Make codebase more self-contained and focused on Python implementation
- Update all notification configuration files
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the notification model was created on tfe folder? Kindly, remove the file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has been resolved

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the resource file under tfe folder. Its not required, as we renamed the folder to pytfe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this has been resolved as well

@isivaselvan isivaselvan merged commit e9082ae into main Oct 15, 2025
11 checks passed
@iam404 iam404 deleted the notifications branch October 28, 2025 06:41
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.

2 participants