sigit/sigit.io
7
2
Fork 0
Code Issues 32 Pull Requests 2 Actions Packages Projects 1 Releases Wiki Activity

Add Sigit ID as a path param #195

Merged
b merged 12 commits from issue-171 into staging 2024-10-07 09:16:30 +00:00
Conversation 5 Commits 12 Files Changed 4 +52 -19
m commented 2024-09-10 14:01:34 +00:00
Owner
Copy Link

Closes #171

Intent

When opening a /sign page, we do not link a particular sigit which we are opening, and we pass the info about sigit in the location.state object.

To make it linkable, I've added an optional :id param in the /sign route and stopped providing state in the location.state.

id param is actually the Sigit ID which is the create event id.

Closes #171 ## Intent When opening a `/sign` page, we do not link a particular `sigit` which we are opening, and we pass the info about `sigit` in the `location.state` object. To make it linkable, I've added an optional `:id` param in the `/sign` route and stopped providing state in the `location.state`. `id` param is actually the `Sigit ID` which is the `create event id`.
m added 1 commit 2024-09-10 14:01:34 +00:00
feat: Add Sigit ID as a path param
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 33s
Details
75a715d002
enes requested changes 2024-09-10 15:53:54 +00:00
Dismissed
src/pages/sign/index.tsx Outdated
@ -73,3 +75,1 @@
arrayBuffer: decryptedArrayBuffer,
uploadedZip
} = location.state || {}
const [metaInNavState, setMetaInNavState] = useState<Meta | undefined>()
enes commented 2024-09-10 15:49:34 +00:00
Member
Copy Link

We should just replace the value of metaInNavState without having states.
We can just move it out of the destructuring and modify it if needed? Something like?

  const { arrayBuffer: decryptedArrayBuffer, uploadedZip } = location.state

  let metaInNavState = location.state.meta
  if (usersAppData) {
    const sigitId = params.id

    if (sigitId) {
      metaInNavState = usersAppData.sigits[sigitId]
    }
  }
We should just replace the value of `metaInNavState` without having states. We can just move it out of the destructuring and modify it if needed? Something like? ``` const { arrayBuffer: decryptedArrayBuffer, uploadedZip } = location.state let metaInNavState = location.state.meta if (usersAppData) { const sigitId = params.id if (sigitId) { metaInNavState = usersAppData.sigits[sigitId] } } ```
m marked this conversation as resolved
src/routes/index.tsx
@ -19,6 +19,7 @@ export const appPrivateRoutes = {
homePage: '/',
create: '/create',
sign: '/sign',
enes commented 2024-09-10 15:06:03 +00:00
Member
Copy Link

Can we use optional segment? Instead of two routes /sign and /sign/:id we can combine it into /sign/:id? .

Can we use optional segment? Instead of two routes `/sign` and `/sign/:id` we can combine it into `/sign/:id?` .
m marked this conversation as resolved
eugene commented 2024-09-11 09:34:49 +00:00
Owner
Copy Link

Can you please add update the description of the PR with its primarily objective and what changes have been made?

Can you please add update the description of the PR with its primarily objective and what changes have been made?
m added 2 commits 2024-09-11 10:12:43 +00:00
style: lint fix
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
7c027825cd
b commented 2024-09-11 10:15:37 +00:00
Owner
Copy Link

I would like to see:

  • more comments (eg, explanation of what the id is)
  • consistent, clear naming convention. sigit, sigitid, id, sigitkey (what are these?) vs sigitCreateId -> selfExplanatory
I would like to see: - more comments (eg, explanation of what the id is) - consistent, clear naming convention. sigit, sigitid, id, sigitkey (what are these?) vs `sigitCreateId` -> selfExplanatory
👍 1
m added 1 commit 2024-09-11 10:29:48 +00:00
chore: comments and lint (typing)
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 33s
Details
86a16c13ce
m added 1 commit 2024-09-11 10:33:48 +00:00
chore: sigitCreateId naming
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 33s
Details
5dc8d53503
enes approved these changes 2024-09-11 11:09:17 +00:00
b reviewed 2024-09-11 11:18:37 +00:00
src/pages/sign/index.tsx Outdated
@ -76,0 +81,4 @@
}
/**
* If userAppData (redux) is available, and we have route param (sigit id)
b commented 2024-09-11 11:18:37 +00:00
Owner
Copy Link

sigit id - what is this? should be defined

sigit id - what is this? should be defined
m marked this conversation as resolved
m added 1 commit 2024-09-11 11:30:51 +00:00
chore: renamed sigitKey to sigitCreateId
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
64e8ebba85
m added 1 commit 2024-09-11 13:41:50 +00:00
chore: comment fix
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
79e14d45a1
m added 1 commit 2024-09-11 15:29:55 +00:00
fix: verify/sign link
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
e48a396990
b added 1 commit 2024-09-13 10:07:27 +00:00
Merge branch 'staging' into issue-171
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
aa8214d015
enes added 1 commit 2024-09-16 12:37:42 +00:00
Merge branch 'staging' into issue-171
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
6ba3b6ec89
b added 1 commit 2024-10-05 21:02:43 +00:00
Merge branch 'staging' into issue-171
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 35s
Details
e33996c1f9
b added 1 commit 2024-10-07 09:15:40 +00:00
Merge branch 'staging' into issue-171
All checks were successful
Open PR on Staging / audit_and_check (pull_request) Successful in 34s
Details
acad24dc06
b merged commit 89971fb176 into staging 2024-10-07 09:16:30 +00:00
b deleted branch issue-171 2024-10-07 09:16:31 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
enes
Labels
Clear labels
Kind/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
Reviewed
Confirmed
Issue has been confirmed
Reviewed
Duplicate
This issue or pull request already exists
Reviewed
Invalid
Invalid issue
Reviewed
Won't Fix
This issue won't be fixed
Status
Abandoned
Somebody has started to work on this but abandoned work
Status
Blocked
Something is blocking this issue or pull request
Status
Need More Info
Feedback is required to reproduce issue or to continue work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sigit/sigit.io#195
No description provided.
Delete Branch "issue-171"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?