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

Time Profile Days/Hours Data Format #21294

Closed
GilbertCherrie opened this issue Jun 23, 2021 · 3 comments · Fixed by #21311
Closed

Time Profile Days/Hours Data Format #21294

GilbertCherrie opened this issue Jun 23, 2021 · 3 comments · Fixed by #21311
Assignees
Labels

Comments

@GilbertCherrie
Copy link
Member

When trying to post data to the Time Profile table the days and hours are being sent as strings where as the api expects symbols :days and :hours. This results in an error when creating a new time profile with a new react form. The days and hours selected on the react form are ignored and the time profile is created with all days and hours selected. Below is an image with the api request data. The second image is the result after the time profile is created and the third image is the result after the time profile is edited.

Screen Shot 2021-06-23 at 11 43 52 AM

Screen Shot 2021-06-23 at 10 43 39 AM

Screen Shot 2021-06-23 at 10 44 48 AM

@Fryguy
Copy link
Member

Fryguy commented Jun 23, 2021

I think this is related to #17201, where the TimeProfile API is presenting the serialized profile column as Strings, but the backend is expecting Symbols. Some possible fixes to move forward

  • Tactical fix: Change the API code to be aware of and write Symbols (or perhaps do this at the profile= method on the model).
  • Medium term fix: Change the backend code to be aware of both Symbols or Strings, or just change the backend to only support Strings with a data migration for existing columns
  • Long term fix: Get rid of the serialized column and use regular columns (perhaps Array columns) for the values, with a data migration. This may introduce backward compatibility issues, so we may need a temporary shim in the API.

@Fryguy Fryguy added the bug label Jun 23, 2021
@GilbertCherrie
Copy link
Member Author

Another API issue, the API currently allows time profiles with duplicate names

@Fryguy
Copy link
Member

Fryguy commented Jun 29, 2021

Another API issue, the API currently allows time profiles with duplicate names

This is a model problem, IMO - there should probably be a validation on the model to not allow duplicates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants