fix: allow non expireable API key

[nityanandagohain]

Allow API keys where expires_at is 0, which means it never expires.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to a497b0c in 1 minute and 9 seconds. Click for details.

• Reviewed 29 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. ee/http/middleware/pat.go:51

• Draft comment:
  Using a sentinel value (time.Unix(0, 0)) to indicate a never‐expiring key is fine; however, ensure this is consistently set when the API key is created.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. pkg/types/factor_api_key.go:70

• Draft comment:
  When expiresAt == 0, the intention is a never‐expiring key. However, expiresAtTime is set via now.AddDate(0, 0, 0) (i.e. now) instead of using the sentinel time.Unix(0,0). Consider explicitly setting expiresAtTime to time.Unix(0,0) when expiresAt is 0.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_INL9ybtFPP4fGBqs

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed 404ce8f in 1 minute and 51 seconds. Click for details.

• Reviewed 227 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/types/factor_api_key.go:87

• Draft comment:
  Using time.Unix(0, 0) to represent a never-expiring API key can be confusing, as it denotes 1970-01-01. Consider defining a clear constant (e.g. NEVER_EXPIRE) and documenting its usage.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While using a constant could make the code slightly more readable, the current implementation is already well-documented with comments. The use of time.Unix(0, 0) is a common pattern in Go for representing special time values. The comment doesn't point out any actual problems or bugs, just suggests a style preference. The suggestion could improve code readability slightly. The current implementation might be confusing to developers not familiar with this pattern. The code already has clear comments explaining the meaning of time.Unix(0, 0), and this is a common enough pattern in Go that it doesn't warrant a special constant. Delete the comment as it's suggesting a style preference rather than pointing out a clear issue, and the current implementation is already well-documented.

2. pkg/types/factor_api_key.go:84

• Draft comment:
  Clarify in the documentation that the 'expiresAt' parameter is in days and that a value of 0 means 'never expires'. This will help avoid potential misinterpretation.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking for clarification in the documentation, which is not allowed as per the rules. It is not a specific code suggestion or a request for a test, and it doesn't address any of the specific additional rules provided.

Workflow ID: wflow_j9ICySt4gbWwrhKI

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 2a2cf48 in 1 minute and 5 seconds. Click for details.

• Reviewed 44 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. ee/http/middleware/api_key.go:51

• Draft comment:
  Ensure the NEVER_EXPIRES constant is correctly used to bypass expiration checks. Using an explicit equality comparison avoids misclassifying non-expiring keys.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. ee/http/middleware/api_key.go:80

• Draft comment:
  Update logging: removed unused formatting verb in error log for clarity.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. pkg/types/factor_api_key.go:13

• Draft comment:
  NEVER_EXPIRES is set to time.Unix(0, 0) to denote a non-expiring key. Since this value is in the past, consider adding documentation to clarify why it's safe to use an equality check despite being before now.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. pkg/types/factor_api_key.go:91

• Draft comment:
  Using the NEVER_EXPIRES constant here (instead of time.Unix(0, 0)) ensures consistency in representing non-expiring API keys.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_uqQyfHwcnvvn6fRL

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 5b21382 in 1 minute and 25 seconds. Click for details.

• Reviewed 33 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. ee/modules/user/impluser/handler.go:218

• Draft comment:
  Good to validate orgID from claims. Consider adding a comment to clarify that this validation supports logic for non-expiring API keys (expires_in_days = 0).
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. ee/modules/user/impluser/handler.go:250

• Draft comment:
  Refetching the created API key ensures any computed fields are included. Verify that this extra lookup is necessary and justified from a performance perspective.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify if the extra lookup is necessary and justified from a performance perspective. This falls under asking the author to confirm their intention or ensure behavior, which is against the rules.

3. ee/modules/user/impluser/handler.go:220

• Draft comment:
  Typographical consistency: The error message refers to "orgId" but the variable is "orgID". Consider updating the error message to use consistent casing (i.e. "orgID").
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about the inconsistency, error messages are user-facing text and don't necessarily need to match variable naming conventions. The current message is perfectly clear to users. This seems like an overly pedantic comment that doesn't meaningfully improve the code. The inconsistency could potentially cause confusion for developers maintaining the code in the future. Having consistent casing throughout the codebase, including error messages, might make the code more maintainable. While consistency is good, this is a very minor stylistic issue in a user-facing error message. The benefit of changing it is minimal and doesn't justify the overhead of a code change. Delete this comment as it suggests a change that provides minimal value and is overly focused on a minor stylistic detail in error message text.

4. ee/modules/user/impluser/handler.go:226

• Draft comment:
  Typographical consistency: The error message refers to "userId" but the variable is "userID". Consider updating the error message to use consistent casing (i.e. "userID").
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_53ppeksQTmZRdvKg

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 48573a9 in 1 minute and 20 seconds. Click for details.

• Reviewed 39 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/types/factor_api_key.go:28

• Draft comment:
  Renaming JSON fields for creator/updater: 'CreatedBy' and 'UpdatedBy' have been changed to 'CreatedByUser' and 'UpdatedByUser'. This is a breaking change for API consumers who expect the old keys.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50% This comment is informative and highlights a breaking change in the API, which is important for the PR author to consider. However, it doesn't ask for any specific action or confirmation from the author, which makes it more of an informative note rather than a suggestion or request for change.

2. pkg/types/factor_api_key.go:139

• Draft comment:
  The conversion function now uses the renamed fields (CreatedByUser/UpdatedByUser) to build the API key output. Confirm that clients consuming this function are updated to handle the new JSON keys.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to confirm that clients consuming the function are updated to handle new JSON keys. This falls under asking the author to ensure behavior is intended or to double-check things, which is against the rules.

Workflow ID: wflow_nPJ5Qn4o4nXSz7zD

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed d0c8c91 in 1 minute and 53 seconds. Click for details.

• Reviewed 427 lines of code in 12 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/api/v1/pats/delete.ts:14

• Draft comment:
  Missing 'return' in the catch block. To consistently propagate errors, return the result of ErrorResponseHandlerV2.
• Reason this comment was not posted:
  Marked as duplicate.

2. frontend/src/api/v1/pats/list.ts:15

• Draft comment:
  In the catch block, the error handler is invoked without a return. Returning its result would ensure consistent error handling.
• Reason this comment was not posted:
  Marked as duplicate.

3. frontend/src/api/v1/pats/update.ts:20

• Draft comment:
  Missing 'return' in the catch block leads to inconsistent error propagation. Consider returning the result of ErrorResponseHandlerV2.
• Reason this comment was not posted:
  Marked as duplicate.

4. frontend/src/container/APIKeys/APIKeys.tsx:350

• Draft comment:
  When converting APIKey.createdAt for date calculations, ensure its format is valid. Since createdAt is defined as a string, an explicit parse (or validation) may be needed to avoid unexpected results.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_wxBJSJ1tj3RudYGe

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[vikrantgupta25]

contributes to - https://github.com/SigNoz/platform-pod/issues/702