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

[BUGFIX inspector] Fix columns names in debug-adapter #5733

Merged
merged 5 commits into from
Nov 5, 2018

Conversation

thorsteinsson
Copy link
Member

@thorsteinsson thorsteinsson commented Nov 1, 2018

Fixed formatting of column desc to support many words. It used to replace only the first underscore with space and leave the rest.

@runspired
Copy link
Contributor

@thorsteinsson so after reviewing this I'm fairly sure that whatever issue is being seen in Ember inspector is not related to ember-data. Ember Data does not and never has allowed users to use a property other than id as the primaryKey field on Model. The primaryKey modification shown in the associated PR is a pre-normalization or post-serialization property name unrelated to the store's internal format or the interface available on Model.

When we interface with the data-adapter we are always dealing with normalized data, and thus will only ever be using id. Either a different data-adapter for a different data-layer is interfacing with the inspector in an incorrect way or there's some other bug involved.

@thorsteinsson
Copy link
Member Author

@runspired Makes sense, I wanted to use the same column name as defined in the model to provide better info in the inspector. I can change that back to id and only fix how the data is returned based on primaryKey.

I've tested the changes with the Inspector and it works. I'll work on modernizing the tests.

Since the DebugAdapter is only used to get debug information (I guess), should be only included in dev builds or be in a separate repo? There's a comment at https://github.com/emberjs/data/blob/master/addon/-private/index.js#L45

@thorsteinsson
Copy link
Member Author

I changed it back to use id and made sure we return a value for when primaryKey has been set (id is not set).

@thorsteinsson thorsteinsson changed the title [BUGFIX inspector] Include primaryKey column and default to 'id' [BUGFIX inspector] Fix columns names in debug-adapter Nov 5, 2018
@runspired runspired merged commit b142626 into emberjs:master Nov 5, 2018
igorT pushed a commit that referenced this pull request Nov 6, 2018
* [BUGFIX inspector] Include primaryKey column and default to 'id'

* [BUGFIX inspector] Always use `id` for primary key

* [CHORE tests] modernize integration/debug-adapter-test

* [BUGFIX inspector] Revert changes to id

* [BUGFIX inspector] Add test for column names
igorT pushed a commit that referenced this pull request Nov 6, 2018
* [BUGFIX inspector] Include primaryKey column and default to 'id'

* [BUGFIX inspector] Always use `id` for primary key

* [CHORE tests] modernize integration/debug-adapter-test

* [BUGFIX inspector] Revert changes to id

* [BUGFIX inspector] Add test for column names
runspired pushed a commit that referenced this pull request Dec 8, 2018
* [BUGFIX inspector] Include primaryKey column and default to 'id'

* [BUGFIX inspector] Always use `id` for primary key

* [CHORE tests] modernize integration/debug-adapter-test

* [BUGFIX inspector] Revert changes to id

* [BUGFIX inspector] Add test for column names
runspired pushed a commit that referenced this pull request Dec 10, 2018
* [BUGFIX inspector] Include primaryKey column and default to 'id'

* [BUGFIX inspector] Always use `id` for primary key

* [CHORE tests] modernize integration/debug-adapter-test

* [BUGFIX inspector] Revert changes to id

* [BUGFIX inspector] Add test for column names
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.

2 participants