Mattermost-server: [MM-15305] Migrate "Preference.CleanupFlagsBatch" to Sync by default

Created on 25 Apr 2019  路  3Comments  路  Source: mattermost/mattermost-server

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.


Notes: Jira ticket

Mattermost is migrating its Store layer to be sync by default, and only use Async when needed and we're looking for contributors to help with that effort. This Help Wanted issue is to migrate the CleanupFlagsBatch in the Preference store.

The expected way to implement it is, go to the Preference store implementation in the store/sqlstore/ directory, modify the method CleanupFlagsBatch to return directly an object from the model module, and a *model.AppError (removing the store.Do wrapper). After that, you must modify the interface defined in store/store.go to match with the changes. Then, you should execute make store-mocks to rebuild the mocks with the new interface. And finally, modify the rest of the code (tests included) to use the new interface of the function properly.

Example: mattermost/mattermost-server#10613

AreTechnical Debt Easy Good First Issue Help Wanted PR Exists TecGo

Most helpful comment

Hi @farhadab, first of all thanks for your interest in the project. About your question, the easiest way is seeing the store function itself. It returns the store.StoreChannel using the store.Do function which receive a function with the result parameter (all that boilerplate is not really important here, because you going to remove that entirely), but the result variable is the key. Inside the function we set the result.Err if there is an error, or the result.Data if there is a result, so, the return type must be the type of the data that you put in that fields.

If you never set the result.Data, you can just return *model.AppError, if you set the result.Data to for example a model.User instance, you should return (*model.User, *model.AppError).

In this specific case, you are returning (int64, *model.AppError).

One important thing, I didn't notice it when I publish the help wanted ticket, but this specific function is only used by the Mattermost enterprise code. We try to avoid help wanted tickets that are only used by enterprise, because normally the community want to help in the open source part. Said that, if you want to keep going with this ticket is ok, if you want to pick other one, is ok too, just let me know because if you don't do it, probably I'll do it and close the ticket to avoid other community members to accidentally pick this one.

All 3 comments

How do I know what model from the model module to return? I'm a prospective new contributor and I'd like to contribute to the sync campaign (epic MM-15277), but as I am new to this project, I'm not clear on what the models that are required in the tickets for this epic are supposed to be. Any additional guidance would be appreciated.

@jespino Could you please take a look at the question? To me it seams like this method is not longer used.

Hi @farhadab, first of all thanks for your interest in the project. About your question, the easiest way is seeing the store function itself. It returns the store.StoreChannel using the store.Do function which receive a function with the result parameter (all that boilerplate is not really important here, because you going to remove that entirely), but the result variable is the key. Inside the function we set the result.Err if there is an error, or the result.Data if there is a result, so, the return type must be the type of the data that you put in that fields.

If you never set the result.Data, you can just return *model.AppError, if you set the result.Data to for example a model.User instance, you should return (*model.User, *model.AppError).

In this specific case, you are returning (int64, *model.AppError).

One important thing, I didn't notice it when I publish the help wanted ticket, but this specific function is only used by the Mattermost enterprise code. We try to avoid help wanted tickets that are only used by enterprise, because normally the community want to help in the open source part. Said that, if you want to keep going with this ticket is ok, if you want to pick other one, is ok too, just let me know because if you don't do it, probably I'll do it and close the ticket to avoid other community members to accidentally pick this one.

Was this page helpful?
0 / 5 - 0 ratings