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

Modify query and store to show subclasses #145

Merged
merged 4 commits into from Jan 14, 2021
Merged

Conversation

guergana
Copy link
Contributor

@guergana guergana commented Jan 11, 2021

first commit: Modify store to accept subclasses
second commit: modify the query.
third commit: merge
fourth commit: this field should be unchecked by default and should be checked true automatically if and only if the user selects a wikibase-item property

@guergana guergana changed the title Modify query and store to also select subclasses Modify query and store to show subclasses Jan 11, 2021
@guergana guergana changed the title Modify query and store to show subclasses [WiP] Modify query and store to show subclasses Jan 11, 2021
it( 'builds a query from a property and a string value with subclasses', () => {
const propertyId = 'P666';
const value = 'blah';
const subclassesId = 'P279';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do we pass the environment variable here? I tried const subclassesId = process.env.VUE_APP_SUBCLASS_PROPERTY; and it didnt work

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could try

process.env = Object.assign(process.env, { VUE_APP_SUBCLASS_PROPERTY: 'value' });

( which I found there: vuejs/vue-test-utils#193 (comment) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uf!! That was hard. The proposed solution wasn't working but this one worked: https://stackoverflow.com/a/58953365/1619792

@guergana guergana force-pushed the select-subclasses-store branch 7 times, most recently from 12e2b51 to 835d801 Compare January 13, 2021 13:08
@guergana guergana changed the title [WiP] Modify query and store to show subclasses Modify query and store to show subclasses Jan 13, 2021
@@ -4194,19 +4194,19 @@
"dependencies": {
"ansi-regex": {
"version": "2.1.1",
"resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz",
"resolved": "https://registry.npm.taobao.org/ansi-regex/download/ansi-regex-2.1.1.tgz",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't want those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes, missed that one

@@ -91,9 +91,10 @@ export default Vue.extend( {
if ( selectedProperty === null ) {
this.$store.dispatch( 'unsetProperty', this.conditionIndex );
return;
} else if ( selectedProperty.datatype === 'wikibase-item' ) {
Copy link
Collaborator

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 need this else as the previous if always returns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if we needed it either.

src/components/QueryCondition.vue Show resolved Hide resolved
package.json Outdated
},
"jest": {
"setupFiles": [
"./tests/unit/setEnvVars.js"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should that not be .jest/setEnvVars.js? I'm confused that the tests are still passing 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this was from another way I tried that didn't work. So this is actually not being used. Thanks for pointing it out.

tests/unit/store/mutations.spec.ts Show resolved Hide resolved
Copy link
Collaborator

@micgro42 micgro42 left a comment

Choose a reason for hiding this comment

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

Thanks! We are almost there 🙂

@@ -4200,13 +4200,13 @@
},
"ansi-styles": {
"version": "2.2.1",
"resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-2.2.1.tgz",
"resolved": "https://registry.npm.taobao.org/ansi-styles/download/ansi-styles-2.2.1.tgz",
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, I mean, we don't want to have this file checked in at all for this PR (and also do not want to use registry.npm.taobao.org)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, weird. I thought I removed this.

src/components/QueryCondition.vue Show resolved Hide resolved
@@ -102,6 +102,26 @@ describe( 'QueryCondition.vue', () => {
expect( store.dispatch ).toHaveBeenCalledWith( 'updateValue', { value: userInput, conditionIndex } );
} );

it( 'set subclasses to true when property type is wikibase-item', async () => {
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 have a test for setting it to false as well if the datatype is string?

@guergana guergana merged commit f965872 into master Jan 14, 2021
@guergana guergana deleted the select-subclasses-store branch January 14, 2021 11:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants