Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: timestamp can’t be null #2133

Merged
merged 16 commits into from
Nov 19, 2024
Merged

fix: timestamp can’t be null #2133

merged 16 commits into from
Nov 19, 2024

Conversation

compojoom
Copy link
Contributor

I don't think that a transaction can have a null timestamp. Looking at what the tx-service returns https://safe-transaction-mainnet.safe.global/#/transactions/safes_all_transactions_list

It seems quite impossible, no?

Same goes for transactionHash.

@compojoom
fix: swagger output concrete module execution type
a596b33
@compojoom
fix: swagger improve TransactionInfo output
8ad767c
@compojoom
fix: swagger output concrete txStatus in Transaction type
51a1e2e
@compojoom
fix: output the concrete queued item type in swagger
6aae302
@compojoom
fix: swagger output concrete tx detail execution type
ffc24d2
@compojoom
fix: swagger output concrete conflictType
146c426
@compojoom
fix: swagger output concrete transfer type
ccad422
@compojoom
fix: swagger improve transferInfo output
bbf9a02
@compojoom
fix: swagger output QueuedItem type
475b2dc
@compojoom
fix: swagger output executionInfo type
a1709e5
@compojoom
fix: swagger output concrete transfer direction
1e9df49
@compojoom
fix: swagger output possible transfer objects for SwapTransferTransac…
b1dec14 
…tionInfo
@compojoom
fix: swapper output concrete transaction entity type
3d6e0bc
@compojoom
fix: swagger output concrete DateLabel type
3ed8837
@compojoom compojoom requested a review from a team as a code owner November 15, 2024 16:57
Base automatically changed from improve-swagger to main November 19, 2024 11:12
@hectorgomezv
Copy link
Member

hectorgomezv commented Nov 19, 2024 •
edited
Loading

It seems quite impossible, no?

The timestamp field is computed from several fields depending on the type of transaction that is being mapped, but you are right, it should never be null given the current mappings. Good catch!

I've rebased the PR and solved a conflict before merging it btw.

@hectorgomezv hectorgomezv merged commit fe38de3 into main Nov 19, 2024
20 checks passed
@hectorgomezv hectorgomezv deleted the timestamp branch November 19, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hectorgomezv hectorgomezv hectorgomezv approved these changes

Assignees

@compojoom compojoom

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@compojoom @hectorgomezv