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
Owner

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
75a715d002
enes requested changes 2024-09-10 15:53:54 +00:00
Dismissed
@ -73,3 +75,1 @@
arrayBuffer: decryptedArrayBuffer,
uploadedZip
} = location.state || {}
const [metaInNavState, setMetaInNavState] = useState<Meta | undefined>()
Member

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
@ -19,6 +19,7 @@ export const appPrivateRoutes = {
homePage: '/',
create: '/create',
sign: '/sign',
Member

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
Owner

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
7c027825cd
Owner

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
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
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
5dc8d53503
enes approved these changes 2024-09-11 11:09:17 +00:00
b reviewed 2024-09-11 11:18:37 +00:00
@ -76,0 +81,4 @@
}
/**
* If userAppData (redux) is available, and we have route param (sigit id)
Owner

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
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
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
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
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
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
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
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.
No reviewers
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sigit/sigit.io#195
No description provided.