Bugfix challenge tags to normal tags after a challenge has ended/deleted

[tsukimi2]

Fixes #12276

Bugfix challenge tags to normal tags after a challenge has ended/deleted and added associated test cases.

Note

When running the newly added test cases, I noticed the following warnings, which I do not have any idea how to fix. Hopefully somebody can point me to the right direction? Thanks.

[Vue warn]: Unknown custom element: - did you register the component correctly? For recursive components, make sure to provide the "name" option.

found in

---> at src/components/tasks/user.vue

[paglias]

Hi @tsukimi2 , I gave a look at the code (thanks for the client side test!) but it seems to simply add a check for tag.challenge being the string version of true - which I don't think ever happens? tag.challenge is either undefined or true as a boolean

[paglias]

Hmm actually there's a bug here... tag.challenge gets converted to a string but should be a Boolean, let me check if there's a reason for that or not

[paglias]

Okay @tsukimi2 , after some internal talks turns out the type of tag.challenge being set to String is a bug and should be changed to Boolean It's also going to need a small migration to convert the strings to booleans. Let me know if you want to handle it in this PR, but if you can't no worries

[tsukimi2]

@paglias Sure, I think I can try handling the change from tag.challenge "string" to "boolean" in this PR, though probably I'll need to wait till the coming weekend to do that. Not sure whether there are other pages that are working currently based on this tag.challenge being a string currently as well. Anything I should pay attention to when trying to fix this bug?

[paglias]

@tsukimi2 thanks! AFAIK there aren't other places where this issue is present because it was treated as a boolean everywhere except that it got saved as a string which caused this bug

[paglias]

The changes are looking good @tsukimi2 , this is just going to need a migration to transform the Strings saved in the database to booleans, let me know if you want to add it or if we should handle it

[tsukimi2]

@paglias Sure, I can add the migration as well.

[paglias]

Left some comments, the main issue is the use of monk but it should be easy to change

[paglias]

could you import the User model

import { model as User } from '../../website/server/models/user';

and use that instead of monk? We stopped using it some time ago, you can see an example here https://github.com/HabitRPG/habitica/blob/develop/migrations/users/full-stable.js#L68

[paglias]

there are two update calls?

[tsukimi2]

Will there ever be a case where challenge = false? If not, then maybe I'll just delete the update on challenge false from the migration script?

[tsukimi2]

@paglias Please note that the most recent push in this PR is not working yet. Just want to ask a question based on the update. Previously, when using mock (instead of mongoose) to run the migration script, it was working fine (challenge field able to change from string to boolean correctly). However, once I changed from using mock to using User model as suggested, the migration script was no longer able to change the challenge field from string to boolean anymore. I tried to issue the update query directly in mongodb and it seems to work perfectly fine. The update query is as follow:

db.users.updateOne({
_id: 'bd95ca4c-8db2-4e8d-8492-b83746b90993'
}, {
$set: {
'tags.$[element].challenge': true,
}
}, {
arrayFilters: [{ 'element.challenge': 'true' }]
});

I tried to debug the problem for quite some time but couldn't figure out what's wrong. Hope someone would provide some useful hints? Thanks.

[paglias]

@tsukimi2 I think there are 2 options here:

1. Since we're using the User model, if you remove the .lean() call when finding the users then the result will be an array of mongoose models and you'll be able to directly change it and then call user.save() instead of making an .update()
2. Use the $[] command documented here with a query like this

db.results.update(
   { "tags.challenge" : "true"},
   { $set: { "tags.$[].challenge": true } },
   { multi: true }
)

and

db.results.update(
   { "tags.challenge" : "false"},
   { $set: { "tags.$[].challenge": false } },
   { multi: true }
)

for false values

[paglias]

@SabreCat This is going to require the migration to be run after the PR is deployed

[paglias]

Took a bit but we made it, thanks @tsukimi2 ! Noted towards your 4th tier