616: (feat) prevent multiple fidelity bonds with same expiry date

[barrytra]

This PR fixes #616 Implementation is done in lockdateForm.js component, that shows a warning message while selecting expiry date whenever a user creates a new fidelity bond. When is warning message displayed? if there already exists fidelity bonds, and a user create a new one. But he selects an expiry date same as in any of the previous bonds. Then the message is displayed warning the user.

[barrytra]

This is how it works. I have two fidelity bonds already made with expiry date august,2024 and october,2025. There is no warning message when i create a new FB with different expiry date. But as i select date october,2025 for my new FB. It gives a warning like this

It works smoothly. The warning vanishes when you set the date to a legal one. Let me know if you see any issues, especially with the warning message(My English is not that good). @theborakompanioni @editwentyone

[editwentyone]

think about, to remove the text inside () and maybe add this comment instead: https://github.com/joinmarket-webui/jam/issues/616#issuecomment-2058529662

[theborakompanioni]

Hey @barrytra! Conecpt ACK!

As discussed, maybe it is a good idea to keep knowledge of Fidelity Bonds out of component LockdateForm. Let me know once you need a review, want a question answered, or need any other help. :pray:

[barrytra]

As required, Information of Fidelity bonds has been moved to CreateFidelityBond file. UX works fine as earlier. Please suggest if any changes required @theborakompanioni

[theborakompanioni]

As required, Information of Fidelity bonds has been moved to CreateFidelityBond file. UX works fine as earlier. Please suggest if any changes required @theborakompanioni

Nice : -) Looks better and better!

Some notes from my side:

• The alert is better placed inside the SelectDate component, instead of LockdateForm - in fact, the LockdateForm component should not need any changes at all.
• Instead of a state variable + function + effect (useState/useEffect), have you considered using useMemo?

e.g.

const [lockDateExists, setLockDateExists] = useState(false)

const ifLockTimeExists = (fidelityBonds: Utxos, lockDate: Api.Lockdate): void => {
    setLockDateExists(false)
    if (lockDate)
      // eslint-disable-next-line array-callback-return
      fidelityBonds.map((fidelityBond) => {
        const locktime = fb.utxo.getLocktime(fidelityBond)
        if (locktime === fb.lockdate.toTimestamp(lockDate)) {
          setLockDateExists(true)
        }
      })
  }

  useEffect(() => {
    if (lockDate) ifLockTimeExists(fidelityBonds, lockDate)
  }, [fidelityBonds, lockDate])

Can be replaced with something like:

const bondWithSelectedLockDateAlreadyExists = useMemo(() => {
  return lockDate && fidelityBonds.some(it => fb.utxo.getLocktime(it) === fb.lockdate.toTimestamp(lockDate))
}, [fidelityBonds, lockDate])

What do you think?

[barrytra]

Ahh yes, that should work and would be much better. And also Alert is shifted to createFidelityBond component. I am working on it

[barrytra]

Suggested changes are made. Now lockdatefrom and fidelitybondSteps components are untouched. This looks fine to me now. what are your suggestions?

[theborakompanioni]

Suggested changes are made. Now lockdatefrom and fidelitybondSteps components are untouched. This looks fine to me now. what are your suggestions?

Awesome!

Now we just have to adapt the message a little bit and we are ready to merge. :rocket:

Great work @barrytra!

[barrytra]

I think everything is covered now. Message has been fixed as well. Is there anything else we need to do?

[theborakompanioni]

I think everything is covered now. Message has been fixed as well. Is there anything else we need to do?

Waiting for comments or approval from @editwentyone - then it is ready to be merged! :rocket: