Skip to content

feat(imis): add iMIS RiSE integration#380

Open
Shubhank-Jonnada wants to merge 16 commits into
masterfrom
sj/imis
Open

feat(imis): add iMIS RiSE integration#380
Shubhank-Jonnada wants to merge 16 commits into
masterfrom
sj/imis

Conversation

@Shubhank-Jonnada

Copy link
Copy Markdown
Contributor

Summary

Adds a new iMIS RiSE integration for managing contacts, events, registrations, groups, tags, media assets, and running custom queries through the iMIS RiSE OData REST API.

Actions (20)

Entity Actions
Contacts list_contacts, get_contact, create_contact
Events list_events, get_event
Registrations list_registrations, get_registration, create_registration, delete_registration
Groups list_groups, get_group, add_group_member, remove_group_member
Tags list_tags, add_tag
Media Assets list_media_assets, get_media_asset
Queries run_query

Auth

Custom auth — users provide site_url, username, password, and client_id. The integration handles OAuth2 password grant token exchange internally.

Validation

  • validate_integration.py — 0 errors, 0 warnings
  • check_code.py — all checks passed
  • check_readme.py — passed
  • check_version_bump.py — passed
  • ✅ 35 unit tests passing
  • ✅ Integration tests included (require IMIS_* env vars)

Applied skills

building-integration, writing-unit-tests, writing-integration-tests

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29ac06dc94

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread imis/imis.py Outdated
if inputs.get("additional_fields"):
body.update(inputs["additional_fields"])

data = await api_request(context, "POST", "EventRegistration", json_data=body)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Post registrations to the RegisterEvent operation

When this action is used to register a contact, iMIS expects the event-registration workflow to be invoked via POST /api/EventRegistration/_execute with an EventRegistrationRequest/OperationName: RegisterEvent payload; posting only EventId and PartyId to the EventRegistration collection skips that workflow and will fail validation for normal events that require registration type, registrant/bill-to, and option-function fields.

Useful? React with 👍 / 👎.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 1fa212c. create_registration now posts to EventRegistration/_execute with OperationName: RegisterEvent and the required PersonData contract shape.

Comment thread imis/imis.py
Comment on lines +281 to +285
body: Dict[str, Any] = {
"FirstName": inputs.get("first_name", ""),
"LastName": inputs["last_name"],
"PrimaryEmail": inputs.get("email", ""),
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use the iMIS PersonData shape when creating contacts

For create_contact, iMIS Party creation expects a PersonData contract with $type and nested PersonName fields rather than top-level FirstName/LastName/PrimaryEmail; with the current default payload, a normal call such as {last_name: "Smith"} does not populate the fields the Party endpoint reads and is likely to be rejected or create an incomplete contact.

Useful? React with 👍 / 👎.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9af1399. create_contact now sends the full PersonData shape with $type and nested PersonName fields instead of top-level FirstName/LastName.

Comment thread imis/imis.py Outdated

data = await api_request(context, "GET", "Party", params=params)
items = data.get("Items", []) if isinstance(data, dict) else []
count = data.get("Count", len(items)) if isinstance(data, dict) else len(items)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Report TotalCount for paged list actions

iMIS paged results expose Count as the number of rows in the current page and TotalCount as the full number of matches, so with limit=20 against a larger contact set this reports count: 20 even though the schema says it is the total number of matching contacts. Use TotalCount when present, and apply the same fix to the other list/query handlers that copied this expression.

Useful? React with 👍 / 👎.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 01ce5db. All list handlers now use data.get('TotalCount', data.get('Count', len(items))) so the full match count is returned, not just the current page size.

Comment thread imis/tests/test_imis_integration.py Outdated
url,
params=params,
json=json,
data=body,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Forward form bodies in the live fetch shim

get_access_token() calls context.fetch(..., data=body) for the form-encoded token request, but this live-test shim only forwards its body parameter to aiohttp and drops data into **kwargs. When IMIS_* credentials are present, the token request is sent without the grant payload, so every integration test fails before reaching the action under test.

Useful? React with 👍 / 👎.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 22e8cef. The real_fetch shim now accepts a data keyword argument and forwards it to aiohttp, so the form-encoded token body is sent correctly.

@TheRealAgentK TheRealAgentK self-requested a review June 22, 2026 08:30

@TheRealAgentK TheRealAgentK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Has open codex comments, no integration test run evidence so far,

@TheRealAgentK TheRealAgentK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One additional docs issue that cannot be placed inline because README.md is not in the diff: the repo-level integrations list does not include the new iMIS integration, so please add an iMIS entry there as well.

Comment thread imis/imis.py Outdated

async def get_access_token(context: ExecutionContext) -> str:
creds = context.auth.get("credentials", context.auth)
site_url = creds.get("site_url", "").rstrip("/")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🚫 Blocker: site_url is user-provided and is used to post the iMIS username/password to /token/ after only rstrip("/"). Please validate this before any credential exchange: require an https scheme, require a hostname, and reject malformed URLs/userinfo. It would also be worth adding format: "uri" and/or an HTTPS pattern/help text in config.json so users cannot accidentally send credentials to plain HTTP or an unintended URL.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3909df3. Added _validate_site_url() that requires HTTPS scheme, a valid hostname, and rejects embedded credentials. Added format: uri to config.json and updated help text.

Comment thread imis/imis.py
async def execute(self, inputs: Dict[str, Any], context: ExecutionContext):
try:
party_id = inputs["party_id"]
data = await api_request(context, "GET", f"Party/{party_id}")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Should-fix: dynamic IDs are interpolated directly into authenticated API paths. If party_id contains /, ?, etc., the workflow could hit an unintended endpoint under the same iMIS host. Please URL-encode path components with quote(str(value), safe="") before interpolation, and apply the same pattern to event, group, registration, and media-asset IDs.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f0d3663. Added _encode_id() using quote(str(value), safe='') and applied it to all dynamic path segments across get_contact, get_event, update_contact, update_event, get_group, remove_group_member, delete_registration, and get_media_asset.

Comment thread imis/README.md

| Action | Description | Key Inputs | Key Outputs |
|--------|-------------|------------|-------------|
| `get_contact` | Get a contact by Party ID | `party_id` | `contact` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Should-fix: list_contacts exists in both config.json and imis.py, but it is missing from this action table. Please add it so the README matches the integration surface.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9d49f41. Added list_contacts row to the README action table.

Comment thread imis/imis.py Outdated
if addr.get("country"):
existing["PrimaryAddress"]["Country"] = addr["country"]
if inputs.get("additional_fields"):
existing.update(inputs["additional_fields"])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Should-fix: merging additional_fields after the canonical fields lets callers override validated fields or system fields. For example, a caller can replace fields this action already controls, and the same pattern appears in create/update event, create registration, and create contact. Prefer explicit supported fields; if this escape hatch stays, merge it first and then set canonical fields, or block overrides of known/system fields.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6143801. additional_fields now filters out Id and $type and is merged after canonical fields are set in create_contact, update_contact, create_event, update_event, and create_registration.

async def test_list_media_assets_live(live_context):
result = await imis.execute_action("list_media_assets", {"limit": 5}, live_context)
assert result.type == ResultType.ACTION, result.result.message
assert isinstance(result.result.data["assets"], list)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Should-fix: the live integration tests only exercise read/list flows, but the integration exposes several mutating actions (create_contact, update_contact, create_event, update_event, create_registration, delete_registration, group membership changes, and tagging). Per the integration-test guidance, please add opt-in @pytest.mark.destructive coverage for at least a safe lifecycle path where cleanup is possible, or document why specific write flows cannot be safely exercised.

@Shubhank-Jonnada Shubhank-Jonnada Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8891178. Added three destructive test flows: create and update contact lifecycle, create and delete registration lifecycle, and add and remove group member lifecycle, all with cleanup.

Changed create_registration to POST to EventRegistration/_execute with
the RegisterEvent OperationName and the required EventRegistrationData
$type contract. Replaced PartyId with ContactId per the API spec.
iMIS requires the full PersonData shape with $type and nested PersonName
object. Replaced flat FirstName/LastName fields with the correct contract.
Count returns the current page size, TotalCount returns the full result
set size. Updated list_contacts, list_events, list_registrations,
list_groups, list_tags, run_query, and list_media_assets.
Added _validate_site_url() that enforces HTTPS, a valid hostname, and
rejects embedded credentials. Applied to both get_access_token and
api_request. Added format: uri and updated help text in config.json.
Added _encode_id() helper using quote(str(value), safe='') and applied
it to all dynamic URL path segments: party_id in get_contact and
update_contact, event_id in get_event and update_event, group_id in
get_group and remove_group_member, registration_id in delete_registration,
and asset_id in get_media_asset.
additional_fields now filters out Id and \$type before merging so callers
cannot overwrite system-controlled contract fields. Applied to
update_contact, create_event, update_event, and create_contact.
The shim was missing the data keyword argument so the form-encoded token
body was never forwarded to aiohttp, causing OAuth token requests to fail.
Changed data=body to data=None and passes data=data or body to aiohttp.
Added three write-and-cleanup tests marked with @pytest.mark.destructive:
contact create and update lifecycle, registration create and delete
lifecycle, and group member add and remove lifecycle.
Added missing list_contacts row to imis/README.md action table. Added
iMIS RiSE section to main README in alphabetical position between
Instagram and Gmail.
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

🔍 Integration Validation Results

Commit: b398d79f0305f0fbd45ae55a8c6f8f12d400a8ae · fix(imis): remove trailing comma from auth.fields in config.json
Updated: 2026-06-23T10:43:38Z

Changed directories: imis

Check Result
Structure ✅ Passed
Code ✅ Passed
Tests ✅ Passed
README ✅ Passed
Version ✅ Passed
✅ Structure Check output
Validating 1 integration(s)...

============================================================
Integration: imis
============================================================
✅ All checks passed!

============================================================
SUMMARY
============================================================
Integrations validated: 1
Total errors: 0
Total warnings: 0

✅ All validations passed!
✅ Code Check output
----------------------------------------
Checking: imis
----------------------------------------

📦 Installing dependencies...

🐍 Checking Python syntax...
   ✅ Syntax OK

📥 Checking imports...
   ✅ Imports OK

📄 Checking JSON files...
   ✅ JSON files OK

🔍 Linting with ruff...
   ✅ Lint OK

🎨 Checking formatting with ruff...
   ✅ Formatting OK

🔒 Scanning for security issues with bandit...
   ✅ Security OK

🛡️ Checking dependencies for vulnerabilities with pip-audit...
   ✅ Dependencies OK

🔗 Checking config-code sync...
   ✅ Config-code sync OK

🔄 Checking fetch patterns...
   ✅ Fetch patterns OK

========================================
✅ CODE CHECK PASSED
========================================
✅ Tests Check output

Integration   Tests  Coverage        Status
-------------------------------------------
imis     35/35       81%      ✅ Passed
-------------------------------------------
Total    35/35            ✅ All passed

✅ Tests passed: imis
✅ README Check output
========================================
✅ README CHECK PASSED
========================================
✅ Version Check output
✅ imis: New integration with version 1.0.0

========================================
✅ VERSION CHECK PASSED
========================================

…lidation

The platform handles credential presence checks itself. Custom auth fields
must only define properties, not a required array.
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