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.

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

fix: alias of page in sidebar did not redirect to the original page #6085

Merged
merged 6 commits into from Aug 11, 2022

Conversation

swk777
Copy link
Contributor

@swk777 swk777 commented Jul 18, 2022

fix #5773

Copy link
Contributor

@llcc llcc left a comment

Choose a reason for hiding this comment

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

LGTM

@llcc llcc requested review from cnrpman and andelf July 19, 2022 04:18
@llcc
Copy link
Contributor

llcc commented Jul 19, 2022

Thanks @swk777 for contributing this PR. Cool workflow.

@swk777 swk777 changed the title fix alias of page in sidebar did not redirect to the original page fix: alias of page in sidebar did not redirect to the original page Jul 22, 2022
@cnrpman cnrpman added :type/bug-fix awaiting-response Issue will be closed if a reply is not received labels Jul 25, 2022
@swk777 swk777 force-pushed the 5773 branch 2 times, most recently from b7e8e11 to 77ab702 Compare July 30, 2022 02:52
@swk777
Copy link
Contributor Author

swk777 commented Aug 3, 2022

'awaiting-response' means needing my response?

@cnrpman
Copy link
Collaborator

cnrpman commented Aug 3, 2022

@swk777 Sorry I didn't notice the updates in the last weekend.

Can you resolve my comment above? There is a potential bug in the current code:

get-original-name would return the name for display. It's different from the sanitized page-name for internal usage (:block/name)

get-alias-source-page return the source page (in sanitized page-name) of an alias. So the returned value can be used for db/entity query and route-handler/redirect-to-page! directly, without calling db-model/get-original-name

@cnrpman cnrpman removed the awaiting-response Issue will be closed if a reply is not received label Aug 3, 2022
@swk777
Copy link
Contributor Author

swk777 commented Aug 3, 2022

@swk777 Sorry I didn't notice the updates in the last weekend.

Can you resolve my comment above? There is a potential bug in the current code:

get-alias-source-page return the source page (in sanitized page-name) of an alias. So the returned value can be used for db/entity query and route-handler/redirect-to-page! directly, without calling db-model/get-original-name

I'll fix it.
I cannot see your comment, is there anything wrong with github?
未命名111

@cnrpman
Copy link
Collaborator

cnrpman commented Aug 3, 2022

@swk777 Sorry I forgot to submit that comment.. They should be shown now.

@swk777
Copy link
Contributor Author

swk777 commented Aug 5, 2022

@swk777 Sorry I forgot to submit that comment.. They should be shown now.

just update, please check

Copy link
Collaborator

@cnrpman cnrpman left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@logseq-cldwalker logseq-cldwalker left a comment

Choose a reason for hiding this comment

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

@swk777 Works great. Thanks! 👍 🚢

@logseq-cldwalker logseq-cldwalker merged commit bfc347a into logseq:master Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When clicked from sidebar, favorited aliases point to their own sheet, not the sheet they're an alias for
4 participants