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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix #7283: customUUID not converted to UUID type #7375

Merged
merged 2 commits into from Nov 21, 2022
Merged

fix #7283: customUUID not converted to UUID type #7375

merged 2 commits into from Nov 21, 2022

Conversation

tennox
Copy link
Contributor

@tennox tennox commented Nov 17, 2022

Took us only about 4 hours to figure out, but we found the fix 馃帀

We found that the only other usage of the custom-uuid argument
in assets / ensure-ref-block! already passes a uuid,
so we chose to convert in the calling api.

Closes #7283

@tennox tennox mentioned this pull request Nov 17, 2022
Closed
@github-actions github-actions bot added the fix label Nov 17, 2022
@logseq-cldwalker
Copy link
Collaborator

Relevant discussion - https://discuss.logseq.com/t/realtime-sync-via-ipfs/12471/5

tiensonqin
tiensonqin reviewed Nov 18, 2022
View reviewed changes
Copy link
Contributor

@tiensonqin tiensonqin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about converting the custom-uuid here if it's a string instead of a UUID?

The function api-insert-new-block! could be used in other places with more APIs.

@tennox
Copy link
Contributor Author

tennox commented Nov 18, 2022 •
edited

How about converting the custom-uuid here if it's a string instead of a UUID?

The function api-insert-new-block! could be used in other places with more APIs.

I actually did it there first, but then we found by debugging that the only other usage of the custom-uuid argument
in assets.cjs / ensure-ref-block! already passes a uuid,
so we chose to convert in the calling api.

But if you still think it's better to convert in the api function (you know the codebase a bit better 馃槄) and can tell us how to handle both cases (string or uuid type passed) - we can for sure change it.

(Disclaimer: I'm very new to clojure world :p)

@gotjoshua
Copy link

gotjoshua commented Nov 18, 2022 •
edited

know how to handle both cases (string or uuid type passed)

Its also a philisophical issue that @tennox and i often dance around...
should the api be strict and throw if it gets the wrong type?
or tolerant and handle multiple types?

@tiensonqin
Copy link
Contributor

should the api be strict and throw if it gets the wrong type?
or tolerant and handle multiple types?

Good suggestion, we'll make some changes.

@tiensonqin tiensonqin merged commit b9f3bee into logseq:master Nov 21, 2022
tiensonqin added a commit that referenced this pull request Nov 21, 2022
@tiensonqin
enhance: api should throw error if invalid block-uuid string
69ef1e7 
Also, make sure UUIDs are converted already when passing to cljs.

Related to #7375.
@tiensonqin tiensonqin mentioned this pull request Nov 21, 2022
Merged
tiensonqin added a commit that referenced this pull request Nov 21, 2022
@tiensonqin
enhance: api should throw error if invalid block-uuid string
d1f39f5 
Also, make sure UUIDs are converted already when passing to cljs.

Related to #7375.
xyhp915 added a commit that referenced this pull request Nov 22, 2022
@xyhp915
Squashed commit of the following:
619df66 
commit 23ec02f
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 14:35:53 2022 +0800

    Make it clear that a UUID string is needed.

commit d1f39f5
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 14:32:49 2022 +0800

    enhance: api should throw error if invalid block-uuid string

    Also, make sure UUIDs are converted already when passing to cljs.

    Related to #7375.

commit 0d64d1a
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 20:38:24 2022 +0800

    fix: CI

commit 2610d39
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 20:34:19 2022 +0800

    fix: lint

commit f87100d
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 18:50:18 2022 +0800

    fix: use capacitor's api to get app's active state

commit 81f90cd
Author: rcmerci <rcmerci@gmail.com>
Date:   Mon Nov 21 18:37:19 2022 +0800

    fix(sync): unify path-normalize and filter out un-normalized files

commit 6c8f674
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 18:19:19 2022 +0800

    fix(iOS): don't pause if it's already resumed from background mode

commit 7911efe
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 18:02:29 2022 +0800

    fix: restore back to the previous impl on pause and add instruments

commit 7ffa3e9
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 17:13:08 2022 +0800

    fix: lint warning

commit 936a534
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 17:06:17 2022 +0800

    fix: sync shouldn't stop when its state is idle

commit 3210d17
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 16:31:49 2022 +0800

    enhance: subscribe to id-token for reactive change

commit 1093fe2
Author: rcmerci <rcmerci@gmail.com>
Date:   Mon Nov 21 14:40:43 2022 +0800

    fix(sync): fix normalize path in <get-remote-all-files-meta

commit 3312bfc
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 13:18:18 2022 +0800

    fix: sync icon is missing

commit 98c8ad3
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Sat Nov 19 08:29:36 2022 +0800

    Load refresh token synchronously

commit 268a783
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Sat Nov 19 08:10:39 2022 +0800

    Instrument all the sync unknown errors

commit a28fed8
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Sat Nov 19 07:56:35 2022 +0800

    Pause sync if there's 404 not found

commit 628377d
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 16:34:19 2022 +0800

    Don't normalize paths when downloading files

commit d4ba355
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 16:11:09 2022 +0800

    fix: local page history not working

commit f47ef56
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 15:59:57 2022 +0800

    Make it clear that watcher events are from mobile

commit 56d6be2
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 15:48:21 2022 +0800

    fix: make sure paths are normalized before uploading to server

commit 17a1ac3
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 13:54:21 2022 +0800

    fix: normalize the paths returned from file watcher

commit 831356e
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 12:55:30 2022 +0800

    Make sure file paths are normalized before writing

commit 671ca3b
Author: rcmerci <rcmerci@gmail.com>
Date:   Fri Nov 18 02:34:34 2022 +0800

    fix(sync): add missed sync-event

commit 65338a9
Author: rcmerci <rcmerci@gmail.com>
Date:   Thu Nov 17 20:14:44 2022 +0800

    fix(sync): apply path-normalize on read/write filepath

commit e9edd91
Author: Peng Xiao <pengxiao@freewheel.tv>
Date:   Sat Nov 19 13:22:42 2022 +0800

    fix: copying whiteboard shapes into outliner

commit b9f3bee
Author: Manu [tennox] <2084639+tennox@users.noreply.github.com>
Date:   Thu Nov 17 12:07:32 2022 +0000

    fix #7283: customUUID not converted to UUID type

    Closes #7283

commit f6ea053
Author: glippi <gabriele@lippi.net>
Date:   Fri Nov 18 17:17:15 2022 +0100

    Add a section on how to contribute by opening a PR

commit 0827a52
Author: Matt Lilley <m.k.lilley@gmail.com>
Date:   Mon Nov 14 22:22:28 2022 +0000

    Fix shift+click on all pages launching a new window

commit 1e09f35
Author: Soumik <griimick@gmail.com>
Date:   Sun Nov 20 05:07:54 2022 +0530

    fix: rebase issue

commit 135717e
Author: charlie <xyhp915@qq.com>
Date:   Tue Nov 15 16:48:27 2022 +0800

    improve: reuse function about filter installed plugins

commit 8e10239
Author: Soumik <10350864+griimick@users.noreply.github.com>
Date:   Wed Nov 9 01:31:23 2022 +0530

    feat: show theme plugin settings under Settings menu

commit 7946847
Author: Gabriel Horner <gabriel@logseq.com>
Date:   Mon Nov 14 16:45:00 2022 -0500

    Fix up rules lint know that rules var is a map

commit 70a476d
Author: Gabriel Horner <gabriel@logseq.com>
Date:   Wed Nov 9 03:17:39 2022 -0500

    Fix up model tests and remove unused rules

commit 96381a9
Author: charlie <xyhp915@qq.com>
Date:   Mon Nov 14 16:46:07 2022 +0800

    enhance(ux): auto width for absolute modal. That's friendly for widen screen.

commit 5abbf8b
Author: Konstantinos Kaloutas <konstantinos@logseq.com>
Date:   Fri Nov 18 14:35:45 2022 +0200

    fix: title bar drag region
xyhp915 added a commit that referenced this pull request Nov 22, 2022
@xyhp915
Squashed commit of the following:
d2e946f 
commit e6fa986
Author: Peng Xiao <pengxiao@freewheel.tv>
Date:   Tue Nov 22 10:48:22 2022 +0800

    fix: skip whiteboard tests for now

commit 7fdb9aa
Author: Zhizhen He <hezhizhen.yi@gmail.com>
Date:   Mon Nov 21 13:56:50 2022 +0800

    fix: typo

commit 584f3b0
Author: Gabriel Horner <gabriel@logseq.com>
Date:   Mon Nov 21 14:30:01 2022 -0500

    Api fns can be called in prod builds with `call-api`

    Follow up to
    #7287 (comment):
    - Replace :classes approach with approach that works in dev and prod
      builds
    - Remove graph-parser as it is unused
    - Make api calling available for all contexts

commit 23ec02f
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 14:35:53 2022 +0800

    Make it clear that a UUID string is needed.

commit d1f39f5
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 14:32:49 2022 +0800

    enhance: api should throw error if invalid block-uuid string

    Also, make sure UUIDs are converted already when passing to cljs.

    Related to #7375.

commit 0d64d1a
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 20:38:24 2022 +0800

    fix: CI

commit 2610d39
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 20:34:19 2022 +0800

    fix: lint

commit f87100d
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 18:50:18 2022 +0800

    fix: use capacitor's api to get app's active state

commit 81f90cd
Author: rcmerci <rcmerci@gmail.com>
Date:   Mon Nov 21 18:37:19 2022 +0800

    fix(sync): unify path-normalize and filter out un-normalized files

commit 6c8f674
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 18:19:19 2022 +0800

    fix(iOS): don't pause if it's already resumed from background mode

commit 7911efe
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 18:02:29 2022 +0800

    fix: restore back to the previous impl on pause and add instruments

commit 7ffa3e9
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 17:13:08 2022 +0800

    fix: lint warning

commit 936a534
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 17:06:17 2022 +0800

    fix: sync shouldn't stop when its state is idle

commit 3210d17
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 16:31:49 2022 +0800

    enhance: subscribe to id-token for reactive change

commit 1093fe2
Author: rcmerci <rcmerci@gmail.com>
Date:   Mon Nov 21 14:40:43 2022 +0800

    fix(sync): fix normalize path in <get-remote-all-files-meta

commit 3312bfc
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Mon Nov 21 13:18:18 2022 +0800

    fix: sync icon is missing

commit 98c8ad3
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Sat Nov 19 08:29:36 2022 +0800

    Load refresh token synchronously

commit 268a783
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Sat Nov 19 08:10:39 2022 +0800

    Instrument all the sync unknown errors

commit a28fed8
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Sat Nov 19 07:56:35 2022 +0800

    Pause sync if there's 404 not found

commit 628377d
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 16:34:19 2022 +0800

    Don't normalize paths when downloading files

commit d4ba355
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 16:11:09 2022 +0800

    fix: local page history not working

commit f47ef56
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 15:59:57 2022 +0800

    Make it clear that watcher events are from mobile

commit 56d6be2
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 15:48:21 2022 +0800

    fix: make sure paths are normalized before uploading to server

commit 17a1ac3
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 13:54:21 2022 +0800

    fix: normalize the paths returned from file watcher

commit 831356e
Author: Tienson Qin <tiensonqin@gmail.com>
Date:   Fri Nov 18 12:55:30 2022 +0800

    Make sure file paths are normalized before writing

commit 671ca3b
Author: rcmerci <rcmerci@gmail.com>
Date:   Fri Nov 18 02:34:34 2022 +0800

    fix(sync): add missed sync-event

commit 65338a9
Author: rcmerci <rcmerci@gmail.com>
Date:   Thu Nov 17 20:14:44 2022 +0800

    fix(sync): apply path-normalize on read/write filepath

commit e9edd91
Author: Peng Xiao <pengxiao@freewheel.tv>
Date:   Sat Nov 19 13:22:42 2022 +0800

    fix: copying whiteboard shapes into outliner

commit b9f3bee
Author: Manu [tennox] <2084639+tennox@users.noreply.github.com>
Date:   Thu Nov 17 12:07:32 2022 +0000

    fix #7283: customUUID not converted to UUID type

    Closes #7283

commit f6ea053
Author: glippi <gabriele@lippi.net>
Date:   Fri Nov 18 17:17:15 2022 +0100

    Add a section on how to contribute by opening a PR

commit 0827a52
Author: Matt Lilley <m.k.lilley@gmail.com>
Date:   Mon Nov 14 22:22:28 2022 +0000

    Fix shift+click on all pages launching a new window

commit 1e09f35
Author: Soumik <griimick@gmail.com>
Date:   Sun Nov 20 05:07:54 2022 +0530

    fix: rebase issue

commit 135717e
Author: charlie <xyhp915@qq.com>
Date:   Tue Nov 15 16:48:27 2022 +0800

    improve: reuse function about filter installed plugins

commit 8e10239
Author: Soumik <10350864+griimick@users.noreply.github.com>
Date:   Wed Nov 9 01:31:23 2022 +0530

    feat: show theme plugin settings under Settings menu

commit 7946847
Author: Gabriel Horner <gabriel@logseq.com>
Date:   Mon Nov 14 16:45:00 2022 -0500

    Fix up rules lint know that rules var is a map

commit 70a476d
Author: Gabriel Horner <gabriel@logseq.com>
Date:   Wed Nov 9 03:17:39 2022 -0500

    Fix up model tests and remove unused rules

commit 96381a9
Author: charlie <xyhp915@qq.com>
Date:   Mon Nov 14 16:46:07 2022 +0800

    enhance(ux): auto width for absolute modal. That's friendly for widen screen.

commit 5abbf8b
Author: Konstantinos Kaloutas <konstantinos@logseq.com>
Date:   Fri Nov 18 14:35:45 2022 +0200

    fix: title bar drag region
@tennox tennox deleted the fix/custom-uuid branch November 25, 2022 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiensonqin tiensonqin tiensonqin left review comments

@xyhp915 xyhp915 Awaiting requested review from xyhp915

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Passing customUUID causes broken UI
4 participants
@tennox @logseq-cldwalker @gotjoshua @tiensonqin