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

feat: import exported boards #4392

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

feat: import exported boards #4392

wants to merge 35 commits into from

Conversation

mateo-ivc
Copy link
Collaborator

@mateo-ivc mateo-ivc commented Aug 5, 2024

Description

Since we have an export feature it would make sense to be able to import those boards again.
Therefore, when creating a new board, there is now an option to import a JSON file that reflects a board.

Changelog

Backend

api/router.go

  • Added import route

api/boards.go

  • Implementation of import board route
  • Parses the json string into the components that will be inserted into the database

dto/...

  • Added structs to parse the JSON data into usable structs.

database/notes.go

  • Added a query to import notes

services/notes.go

  • Added import method for notes -> calls importNote in the DB layer.

Frontend

api/boards.ts

  • Implementation of importBoard route -> sends json string

Note/NoteAuthorList

  • Added authorID of note to be able to create a avatar prop based on the authorID

PassphraseModal

  • Popup that asks for a password when importing a previously password-protected board.

constants/name.ts

  • Since imported Notes with the same author should have the same avatar and name, i implemented a function that returns a random name with seed

NewBoard

  • Added a new button to import a board by drag and drop or by selecting a file.
  • Asks for password if board was prior password protected

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • The light- and dark-theme are both supported and tested
  • The design was implemented and is responsive for all devices and screen sizes
  • The application was tested in the most commonly used browsers (e.g. Chrome, Firefox, Safari)

(Optional) Visual Changes

@mateo-ivc mateo-ivc self-assigned this Aug 5, 2024
@mateo-ivc mateo-ivc marked this pull request as ready for review August 14, 2024 07:28
@mateo-ivc mateo-ivc changed the title Mi/import board feat: import exported boards Aug 14, 2024
@Schwehn42
Copy link
Collaborator

@mateo-ivc is this ready?

@mateo-ivc
Copy link
Collaborator Author

@mateo-ivc is this ready?

I think the implementation is done, but I should add some tests


if (response.status === 201) {
const body = await response.json();
return body.id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you cast the returned body to its respective type?

Comment on lines +19 to +21
background: #ffffff;
border-radius: 0.5em;
box-shadow: 0 10px 20px rgba(black, 0.2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use colors from colors.scss instead of hardcoded hex values or named standard colors (white being $gray--000, for example).

Comment on lines +22 to +26
left: 50%;
max-width: 90%;
position: absolute;
top: 50%;
transform: translate(-50%, -50%);
Copy link
Collaborator

Choose a reason for hiding this comment

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

so I'm assuming you centered the container here. Can you make the the parent container flex instead and use align-items and justify-content instead, or is there any specific reason to do it this way?

Comment on lines +51 to +52
background: $color-form-background--dark;
color: white;
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, both are hardcoded colors

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is so the other authors keep the same name, right? This is why I was a proponent of keeping the real names, but oh well can't do anything about it 😅

Please move this to utils/random though

completeBoard.board.passphrase = passphrase;
}

const boardId = await API.importBoard(JSON.stringify(completeBoard));
Copy link
Collaborator

Choose a reason for hiding this comment

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

direct API calls like that are not in the spirit of our architecture. Please use a thunk for this

@Schwehn42 Schwehn42 added the Changes Requested Changes requested by the reviewer label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Requested Changes requested by the reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants