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

Add orchestra api for todo templates and starting order #736

Merged
merged 153 commits into from Jan 25, 2021

Conversation

adbharadwaj
Copy link
Collaborator

@adbharadwaj adbharadwaj commented Jan 20, 2021

Added three orchestra API:

  • Get todo templates
    • Returns list of todo templates
  • Create todos from a template
    • Input is project id, template slug, and step slug
    • Returns a dictionary with keys success=True and todos=List of newly created todos
  • Add step id field to the list of steps returned with get project information api

@coveralls
Copy link

coveralls commented Jan 20, 2021

Coverage Status

Coverage increased (+0.07%) to 94.433% when pulling 8259c2e on todo-template-api into 90f7c01 on main.

permissions=(IsSignedUser,),
logger=logger,
auths=(OrchestraProjectAPIAuthentication,))
def todo_sections_starting_order(request):
Copy link
Member

Choose a reason for hiding this comment

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

@jumasheff is doing the code review, so i'll just focus on this one endpoint to make sure I understand its purpose.

I can't think of an experience in Orchestra or the B12 product that requires this order from Orchestra, as the product currently decides on the fixed section order on its end. Since Orchestra doesn't model sections deeply (maybe it should---right now they are just strings), it seems like the metadata for the section order doesn't have a great home in Orchestra without more data modelling.

Because of the lack of model, the implementation here orders sections by count, which is different from the behavior in the product. I'm open to ideas, but I can imagine two worlds

  • Make it so sections are just strings in Orchestra. The caller (e.g., B12 product) can determine their display order.
  • Push section metadata like section order into Orchestra's data model for to-do lists, and as a result, add that information to the existing to-do endpoints.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got rid of the API.

@@ -581,6 +587,7 @@ def test_update_todos_from_todolist_template_success(self):
project=self.project.id,
step=self.step.slug),
]
print(expected_todos)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove print

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed :)

}


@api_endpoint(methods=['POST'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why POST, and not GET?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got rid of the API itself.

authentication_classes = (OrchestraProjectAPIAuthentication,)

serializer_class = TodoListTemplateSerializer
queryset = TodoListTemplate.objects.all()
Copy link
Contributor

Choose a reason for hiding this comment

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

queryset = TodoListTemplate.objects.filter(is_deleted=False)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we will need this because we are inheriting from BaseModel which inherits from BaseModelManager.

class TodoListTemplate(TodoListTemplateMixin, BaseModel):

class BaseModel(DeleteMixin, models.Model):
"""
Abstract base class models which defines created_at and is_deleted fields.
Attributes:
created_at (datetime.datetime):
Datetime at which the model is created.
is_deleted (boolean):
If value is True, mdoel is deleted. Default is False.
"""
objects = BaseModelManager()
unsafe_objects = models.Manager()

class BaseModelManager(models.Manager):
"""
Model manager intended to be used with models with an `is_deleted` field.
Overrides the initial QuerySet to filter for objects where `is_deleted`
is false.
"""
def get_queryset(self):
return super().get_queryset().filter(is_deleted=False)

BaseModelManager sets the default query to is_deleted=False

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just to be sure, I confirmed by deleting a todolist template and made sure that we don't need is_deleted=False

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow! Ignore my suggestion. A!

@adbharadwaj adbharadwaj merged commit 29e5ffe into main Jan 25, 2021
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