Skip to content

Conversation

@TomAlbertInvent
Copy link
Contributor

No description provided.

@prasad-albert
Copy link
Collaborator

Thanks for the PR @TomAlbertInvent! Mind dropping a quick summary of the changes so I can review faster? :)

@TomAlbertInvent
Copy link
Contributor Author

Thanks for the PR @TomAlbertInvent! Mind dropping a quick summary of the changes so I can review faster? :)

Sure! Here's the changes I made. Added merge companies
Added create custom templates
Add required parameters
Added Custom Template Viewer ACL
Added Medium Priority to Custom Templates
Added Notes to GeneralData for Custom Templates
Added Notes and Tags to Batch and Property Data for Custom Templates
Added PROCESS to sheets
Added a self check to sheets to avoid adding multiple inventory rows of the same item
Added ID to NotebookData in Custom Templates

@TomAlbertInvent
Copy link
Contributor Author

Thanks for the PR @TomAlbertInvent! Mind dropping a quick summary of the changes so I can review faster? :)

Sure! Here's the changes I made. Added merge companies Added create custom templates Add required parameters Added Custom Template Viewer ACL Added Medium Priority to Custom Templates Added Notes to GeneralData for Custom Templates Added Notes and Tags to Batch and Property Data for Custom Templates Added PROCESS to sheets Added a self check to sheets to avoid adding multiple inventory rows of the same item Added ID to NotebookData in Custom Templates

@prasad-albert

sources: list[TaskSource] | None = Field(alias="Sources", default=None)
parent_id: str | None = Field(alias="parentId", default=None)
notes: str | None = Field(default=None)
tags: list[Tag] | None = Field(default=None, alias="Tags")
Copy link
Collaborator

Choose a reason for hiding this comment

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

GeneralData inherits from BaseTaggedResource which has tags in it, so we don't need it here.

priority: Priority # enum?!
workflow: list[EntityLink] = Field(default=None, alias="Workflow")
notes: str | None = Field(default=None)
tags: list[Tag] | None = Field(default=None, alias="Tags")
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

project: SerializeAsEntityLink[Project] | None = Field(alias="Project", default=None)
inventories: list[DataTemplateInventory] | None = Field(default=None, alias="Inventories")
due_date: str | None = Field(alias="dueDate", default=None)
tags: list[Tag] | None = Field(default=None, alias="Tags")
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

],
)

def create(self, *, custom_template: CustomTemplate) -> CustomTemplate:
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think the API allows creation of multiple customtemplates at once, we could do something like this

Suggested change
def create(self, *, custom_template: CustomTemplate) -> CustomTemplate:
def create(self, *, custom_templates: list[CustomTemplate]) -> list[CustomTemplate]:

Comment on lines 91 to 112
"""Creates a new custom template.
Parameters
----------
custom_template : CustomTemplate
The custom template to create.
Returns
-------
CustomTemplate
The created CustomTemplate object.
"""

response = self.session.post(
url=self.base_path,
json=[
custom_template.model_dump(
mode="json", by_alias=True, exclude_unset=True, exclude_none=True
)
],
)
return CustomTemplate(**response.json()[0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"""Creates a new custom template.
Parameters
----------
custom_template : CustomTemplate
The custom template to create.
Returns
-------
CustomTemplate
The created CustomTemplate object.
"""
response = self.session.post(
url=self.base_path,
json=[
custom_template.model_dump(
mode="json", by_alias=True, exclude_unset=True, exclude_none=True
)
],
)
return CustomTemplate(**response.json()[0])
"""Create new custom templates.
Parameters
----------
custom_templates : list[CustomTemplate]
The custom templates to create.
Returns
-------
list[CustomTemplate]
The list of created CustomTemplate objects.
"""
if isinstance(custom_templates, CustomTemplate):
custom_templates = [custom_templates]
response = self.session.post(
url=self.base_path,
json=[
custom_template.model_dump(
mode="json", by_alias=True, exclude_unset=True, exclude_none=True
)
for custom_template in custom_templates
],
)
return [CustomTemplate(**x) for x in response.json()]

what do you think about this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: why explicitly exclude unset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That looks good to me!



@pytest.fixture(scope="session")
def seeded_custom_templates(client: Albert, seed_prefix: str) -> Iterator[list[Location]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we remove this function and generate_custom_template_seeds ?
and just define data=GeneralData(name=f"{seed_prefix}-general") instead of fetching from the seed?

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.

4 participants