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

Hook Mongo Cluster onto Server backend #20

Closed
3 tasks done
waltervan00 opened this issue Aug 24, 2021 · 6 comments · Fixed by #32
Closed
3 tasks done

Hook Mongo Cluster onto Server backend #20

waltervan00 opened this issue Aug 24, 2021 · 6 comments · Fixed by #32
Assignees
Labels
help wanted Extra attention is needed
Milestone

Comments

@waltervan00
Copy link
Collaborator

waltervan00 commented Aug 24, 2021

As @pep1032314 had created an Atlas Cluster, we would like to hook it into the back-end code.

This requires following the process on Atlas to obtain a connection string, with appropriate database credentials.

  • Please do not disclose database access credentials here, or in the source code. Spread it via email and refer to it in the connection code through environmental variables. This environmental variable should also be sent to the deployment platform.

  • The infrastructure is set up and the back-end is able to establish a connection to the cluster

  • Implement updated models. See MongoDB object style UML #16 for discussion

Branch db connection has been created for this purpose

@waltervan00 waltervan00 added the help wanted Extra attention is needed label Aug 24, 2021
@waltervan00
Copy link
Collaborator Author

Upon further inspection, server connection string can be passed in as a program argument. Perhaps this can be used to bypass needing to hide the connection string in the source code. We will need to confirm that this can be done in deployment platform.

@waltervan00 waltervan00 added this to the Sprint 1 milestone Aug 24, 2021
@waltervan00 waltervan00 pinned this issue Aug 24, 2021
@waltervan00
Copy link
Collaborator Author

@shangzhel
Regarding the MongoDB access string, where should a good place to place it? I intend on using .env package to store the database link on the source files while hiding the credientials. For example

const uri = "mongodb+srv://<usrname>:<password>@<links>;
const client = new MongoClient(uri, { useNewUrlParser: true, useUnifiedTopology: true });

I understand that with the setup of npm, we are able to pass in arguments for the connection string. Shall can this be set up in deployment? #18

@shangzhel
Copy link
Collaborator

We don't put the access string anywhere in the repository, and we don't need the env package. It will be added on the deployment host as an config/environment variable.

@waltervan00 waltervan00 unpinned this issue Aug 24, 2021
@waltervan00 waltervan00 linked a pull request Aug 25, 2021 that will close this issue
@waltervan00 waltervan00 reopened this Aug 26, 2021
@waltervan00
Copy link
Collaborator Author

waltervan00 commented Aug 26, 2021

Looking at the cards Schema, card Id is not defined in the current repo. Assuming that cards are stored as an array inside each user, we would have to either use the index of the cards in the array or explicitly define a cardsId and using the ObjectId constructor to initialise new cards. This concerns #23 #26 #25, and has a contradiction with the api specifications where a CardId is expected.

The explicit definition of _id inside cardSchema can be reflected in the typescript interface. But calling _id will trigger the linter. So another path name could be considered.

@chomosuke
Copy link
Owner

If you have a look at docs/api/Card.ts you'll see that card Id has the type ObjectId, recall when shangzhe said in a certain meeting, each document by default has an ObjectId attached to it. Using this ObjectId is particularly convenient because it is guaranteed to be unique by the database.

Does this address your concern?

@waltervan00
Copy link
Collaborator Author

Yes it does. I'll see what I can do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants