Skip to content

Conversation

@EliuX
Copy link
Contributor

@EliuX EliuX commented Apr 9, 2020

In this PR it is installed and configured a library to run migrations. In addition, it creates an initial migration that creates an initial database schema. With containers for:

  • Project
  • Project type
  • Activity
  • Customer
  • Time entry

Although the library for migrations we are using is not perfect, we were able to run the migrations and persist them in Azure Cosmos DB.

'id': 'project_type',
'partition_key': PartitionKey(path='/customer_id'),
'unique_key_policy': {
'uniqueKeys': [
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to include parent_id and maybe also tenant_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tenant id is the parent partition_key, so it kind of defines a domain for those unique keys (it is kind of unique by default). The parent_id cannot be unique, because this is a tree structure: every branch has multiple branches or leaves not only one.

container_definition = {
'id': 'project',
'partition_key': PartitionKey(path='/tenant_id'),
'unique_key_policy': {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need project_type and maybe customer_id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you can actually have multiple projects with the same type in a same tenant. That is not a problem.

'partition_key': PartitionKey(path='/tenant_id'),
'unique_key_policy': {
'uniqueKeys': [
{'paths': ['/owner_id', '/end_date']},
Copy link
Contributor

Choose a reason for hiding this comment

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

These two fields belongs to the model. Why is this path different form the others to define uniqueKeys. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because here the uniqueness is the combination owner_id and end_date. This means that there cannot be 2 entries with the same end_date. Having into consideration that null is also a value, then if by mistake the client sends the creation of a time entry twice, as the second one will also have the end_date as null there will be a uniqueness issue and it will not be created. This way the client will be forced to close the existent opened time entry to create a new one.

'id': 'time_entry',
'partition_key': PartitionKey(path='/tenant_id'),
'unique_key_policy': {
'uniqueKeys': [
Copy link
Contributor

Choose a reason for hiding this comment

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

Following the same observation of models above. I think here we need project_id, activity_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you can be working in that same project_id and doing that same activity id even with different technologies or even the same combination of the 3 of them. For instance, you are working at something and then you change to the standup and when you finish you go back to a time entry with the same data as before.

Copy link
Contributor

@Angeluz-07 Angeluz-07 left a comment

Choose a reason for hiding this comment

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

Nice job. Only some observations around container definitions. The fields where I said maybe is because maybe the uniqueness constraint It could be already handled by the nature of the PartitionKey.

@EliuX EliuX merged commit 6571f32 into master Apr 13, 2020
@EliuX EliuX deleted the feature/add-migrations-support#36 branch April 13, 2020 17:09
@EliuX EliuX self-assigned this Apr 17, 2020
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.

3 participants