Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Rename workitem field names #2177

Closed
DhritiShikhar opened this issue Jul 24, 2018 · 10 comments
Closed

Rename workitem field names #2177

DhritiShikhar opened this issue Jul 24, 2018 · 10 comments

Comments

@DhritiShikhar
Copy link
Contributor

DhritiShikhar commented Jul 24, 2018

As per discussion with @kwk

Expected

As per JSON API SPECIFICATION, A period (.) should not be used as a member name.

Actual

Currently, almost all the workitem field member name contain a period:

	SystemRemoteItemID        = "system.remote_item_id"
	SystemNumber              = "system.number"
	SystemTitle               = "system.title"
	SystemDescription         = "system.description"
	SystemDescriptionMarkup   = "system.description.markup"
	SystemDescriptionRendered = "system.description.rendered"
	SystemState               = "system.state"
	SystemAssignees           = "system.assignees"
	SystemCreator             = "system.creator"
	SystemCreatedAt           = "system.created_at"
	SystemUpdatedAt           = "system.updated_at"
	SystemOrder               = "system.order"
	SystemIteration           = "system.iteration"
	SystemArea                = "system.area"
	SystemCodebase            = "system.codebase"
	SystemLabels              = "system.labels"
	SystemBoardcolumns        = "system.boardcolumns"

	SystemBoard = "Board"

Proposal

1. Create a middleware in backend which would convert both e.g. system.area as well as area to store workitem field name as area in DB.

2. Write a migration script to rename workitem fields for existing workitems
See #2177 (comment)

  1. Add support for system_* along with system.* fields in backend. We will deal with only system_* fields in backend. All the request/response payload will be modified to support backward compatibilty. Tracked via Support system.* fields and system_* fields in backend #2343
  2. Migration of the clients. The clients will be moved from system.* fields to system_* fields.
  3. Drop support for system.* fields and remove all the temporary code added to support backward compatibilty

Related Issues

#2029
#2028
#2181

@kwk
Copy link
Collaborator

kwk commented Jul 24, 2018

Related issues are:

#2029
#2028

@jarifibrahim
Copy link
Member

@DhritiShikhar @kwk Correct me if I am wrong, system.area would be area, system.iteration would be iteration and so on.

@DhritiShikhar
Copy link
Contributor Author

@jarifibrahim yes

@kwk
Copy link
Collaborator

kwk commented Jul 25, 2018

@jarifibrahim here (#2028 (comment)) you wrote that we need to be able to distinguish between custom and user defined fields. Most likely the field name will never be shown to the user in the UI once we allow the user to add new fields. So for user defined fields we might as well just give them UUIDs as field names.

But for now when we manually deal with field names in all of our tests, it is nice to know when something is a field coming from the planner item type or from a custom field defined in a test fixture or so. So I'd say let's keep at least any prefix, it could be sys_, system_, or something else.

@jarifibrahim
Copy link
Member

So I'd say let's keep at least any prefix, it could be sys_, system_, or something else.

@kwk I agree with this. But @DhritiShikhar's proposal in #2177 (comment) would remove system/sys prefix.

@DhritiShikhar
Copy link
Contributor Author

DhritiShikhar commented Jul 25, 2018

@kwk @jarifibrahim I need to think more on this. Why would you want to distinguish between a field name created through test fixture and a system defined field? And even if you would want to do so, why would you want the client to pass request for a workitem with fields having prefix system.something?

@DhritiShikhar
Copy link
Contributor Author

Also, I have created an issue related to naming convention here #2181

@kwk
Copy link
Collaborator

kwk commented Jul 25, 2018

@kwk @jarifibrahim I need to think more on this. Why would you want to distinguish between a field name created through test fixture and a system defined field?

I don't want to do that. But we have tests in place that define a custom field in a work item type and they use the test fixture. That's why I've brought it up.

And even if you would want to do so, why would you want the client to pass request for a workitem with fields having prefix system.something?

Clients will most likely not hand written fields, so why should they care how things are named?

@jarifibrahim
Copy link
Member

jarifibrahim commented Oct 25, 2018

Renaming of fields involves changes to the payload, the data stored in the database and the migration of the client. This is a multi-step process. So here's how we're going to do this

Step 1: Migration of the Backend
We migrate the backend in this phase and add some temporary code which would allow backward compatitibilty. This would mean the clients can migrate gracefully without any loss of functionality.
By the end of this phase, the backend will accept both system.* and system_* field names but internally it will use only system_* field names. The response will also contain both type of field names.
We will also migrate the database in this step.

For Workitem Create/Update => The request payload can have system. field or system_ field.
For Workitem Show/List or Workitem Type Show/List => The response will contain both system. fields and system_ fields. So if the payload has X number of system. fields, there will be X number of system_ fields

Step 2: Migration of the Client
In this phase the backend will accept and return both kinds of field names. The client can easily switch from one type to another. We are aware of only three clients as of now. The fabric8-planner, fabric8-analytics and fabric8-test (e2e tests). Once all the three clients are migrated, we will remove the temporary code.

Step 3: Removal of the Temporary Code
Once all the clients have migrated, we will remove the temporary code added for backward compatibilty. By the end of this phase, the backend will no longer send or accept system.* field names. We will support only system_* field names.

@jarifibrahim
Copy link
Member

Closing this because of #2401 (comment)

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

Successfully merging a pull request may close this issue.

3 participants