Allow check-ins to be made by slug as well as id.

[halfbyte]

Follow up to #508 - As agreed with @subzero10 we've left this change out of an already big PR

Edit:

TODO

• Allow calling check-in method with a slug. To check-in using a slug, you should construct a URL as described here.
• Make slug a required field and validation should fail if slug is not provided

[joshuap]

Is this issue to add something like this? https://github.com/honeybadger-io/honeybadger-js/blob/6e18f2f8a31a4f5ae698f3c9652a02b124531b79/packages/js/src/server.ts#L97

I don't really love that feature in the other client libraries, tbh, because it introduces an extra request that must be made before each check in request—but the majority of people will not be using the check-in name. Also, we added the slug option—so why do we need to support the name at all? cc @subzero10

[subzero10]

but the majority of people will not be using the check-in name

Can you elaborate on this? Why would the not be using the check-in name? If people start defining check-ins with through code, they won't interact at all with check-in ids.

because it introduces an extra request that must be made before each check in request

Yeah, I don't love this either. And there are multiple ways we could improve it, though I didn't consider to dive into it from the first iteration. But, depending on the language and platform, this is not necessarily true. For example, in JS (and Ruby?), we are storing the check-in id in-memory, so the next time we make the check-in call, we won't have to make a request to get the id. For PHP however, since the lifecycle of a process usually starts and ends with the request, the request would most probably be made every time.

Also, we added the slug option—so why do we need to support the name at all?

That's a good question. I initially considered using slugs for identifiers, but I didn't go for it because they are optional. But, it just occurred to me that we can mark them them as required when using the check-ins configuration api.

[joshuap]

That's a good question. I initially considered using slugs for identifiers, but I didn't go for it because they are optional. But, it just occurred to me that we can mark them them as required when using the check-ins configuration api.

The thing I like about the slug, is that—assuming they are unique per project—you don't need to query for them at all. They're just an alternative identifier in that case. I also think it's a more natural assumption that changing the slug would break existing check-ins, and it's a less natural assumption that changing the name would break existing check-ins. Until now, the name was more of a label that we use to identify the check-in in the UI. But I could be missing something here, I've mostly been following this from afar. cc @stympy

[stympy]

The thing I like about the slug, is that—assuming they are unique per project—you don't need to query for them at all. They're just an alternative identifier in that case. I also think it's a more natural assumption that changing the slug would break existing check-ins, and it's a less natural assumption that changing the name would break existing check-ins. Until now, the name was more of a label that we use to identify the check-in in the UI.

I agree on this. While both the name and the slug are currently optional, and while they both (now) must be unique per project, I think it makes sense to lean more on the slug than the name as an identifier.

[subzero10]

OK then, we can go ahead and use slugs instead. Actually, I like this approach better!

• @halfbyte I will update the issue name and description on what I think should be done to support this.
• On my part, I will get started right away with the other client libraries because this is a breaking change and I want to avoid a major version release (hopefully we haven't had users adopting it yet - though it'd be nice if there was a way to know that already).

[joshuap]

OK then, we can go ahead and use slugs instead. Actually, I like this approach better!

• @halfbyte I will update the issue name and description on what I think should be done to support this.
• On my part, I will get started right away with the other client libraries because this is a breaking change and I want to avoid a major version release (hopefully we haven't had users adopting it yet - though it'd be nice if there was a way to know that already).

@subzero10 since we're requiring slug to be present, can we also use slug instead of the name when syncing check-ins? I think it makes sense that you can change the name without consequences to syncing, and the slug seems like an obvious alternative, since it should be required and unique per-project. Let me know if that makes sense! If we can get this ironed out, I agree with updating the PHP and JS libs before people start using the current implementation too much.

[joshuap]

Once we have the issues around name, slug, and project_id ironed out, I'll feel more comfortable returning to #508.

[subzero10]

@subzero10 since we're requiring slug to be present, can we also use slug instead of the name when syncing check-ins? I think it makes sense that you can change the name without consequences to syncing, and the slug seems like an obvious alternative, since it should be required and unique per-project. Let me know if that makes sense! If we can get this ironed out, I agree with updating the PHP and JS libs before people start using the current implementation too much.

Ah yes, definitely, that's the reason why we will make slug a required field when using the check-ins configurations api. I forgot to write it in #508 😅.