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

Link to page block #73

Merged
merged 5 commits into from
Sep 15, 2022
Merged

Link to page block #73

merged 5 commits into from
Sep 15, 2022

Conversation

idolcemia
Copy link
Contributor

@idolcemia idolcemia commented Sep 14, 2022

New unit test for blocks of type link_to_page.

Such blocks can be of two types: page or database. As both types of expansions use the same procedure, a "database" version of this unit test would be redundant.

Link to the issue:
#66

- Test for page expansion on link to page.
- Test for page expansion on link to page.
- Test for page expansion on link to page.
@idolcemia idolcemia marked this pull request as ready for review September 14, 2022 21:10
tests/test_blocks.py Outdated Show resolved Hide resolved
'strikethrough': False,
'underline': False,
'code': False,
'color': 'default'},
Copy link
Contributor

Choose a reason for hiding this comment

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

See other comment about trailing parens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. And the mock_rich_text_array() call took care of it anyway.

n2y/notion_mocks.py Outdated Show resolved Hide resolved
n2y/notion_mocks.py Outdated Show resolved Hide resolved
Copy link
Contributor

@johndgiese johndgiese left a comment

Choose a reason for hiding this comment

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

@idolcemia nice work! I had a few small comments that need to be addressed, but it's very close to done!

- Simplify notion_mocks.py method mock_page
- Various stylistic changes
@idolcemia
Copy link
Contributor Author

image

@johndgiese
Copy link
Contributor

@idolcemia we didn't really have a mechanism for tracking bugs within Notion (in the past we've used GitHub Issues with the "bug" label). I added a "Tags" property to the "Tasks" database and moved the bug you wrote up into one. See https://www.notion.so/innolitics/Uncaught-Error-Embedded-Page-ea10752d0c0345069443f07860b10282

Copy link
Contributor

@johndgiese johndgiese left a comment

Choose a reason for hiding this comment

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

Nice work @idolcemia thanks for addressing all of the comments. Feel free to merge!

I'll write up another issue for you shortly.

@idolcemia idolcemia merged commit 1899fff into main Sep 15, 2022
@idolcemia idolcemia deleted the link-to-page-block branch September 15, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants