Remove unused status flags from https://github.com/mattermost/mattermost-redux/blob/master/src/reducers/requests/admin.js
We have a pattern to track API requests success, failure and progress states by making respective redux actions like
export function getRolesByNames(rolesNames: Array<string>) {
return bindClientFunc({
clientFunc: Client4.getRolesByNames,
onRequest: RoleTypes.ROLES*BY_NAMES*REQUEST,
onSuccess: <RoleTypes.RECEIVED*ROLES, RoleTypes.ROLES_BY_NAMES*SUCCESS],
onFailure: RoleTypes.ROLES*BY_NAMES*FAILURE,
params: [
rolesNames,
>,
});
}
which modify the redux state of requests object
export default (combineReducers({
getRolesByNames,
getRoleByName,
getRole,
editRole,
}): (RolesRequestsStatuses, GenericAction) => RolesRequestsStatuses);
Most of these state objects are not needed because we don't consume them either in webapp or in mobile app repo.
The proposed solution is to remove any dispatches with success, failure or progress along with the reducer and initial state entity by checking their dependency in both webapp and mobile app repos.
If the redux state for the request is used only in one component we can use react state instead of redux so, we can remove the entity
PR's for reference:
Redux PR: https://github.com/mattermost/mattermost-redux/pull/748
Mobile PR: https://github.com/mattermost/mattermost-mobile/pull/2490
Web PR: https://github.com/mattermost/mattermost-mobile/pull/2496
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.
Taking a stab at this.
@mbluemer Hey, How is this going? Is there anything i can help you with? Fell free to mention me if you need any help with the issue.
Hey @sudheerDev and @amyblais , sorry about the delay but it seems that life right now has gotten ahead of ambition to contribute, I haven't had the spare time to get to this. Opening it up for someone else to pick up.
I'll give it a whirl @hanzei
Awesome, thanks @Mycobee :+1:
Just a heads up...I am busy through the end of the week, and plan to have this PR open by the end of the weekend.
Thanks for your patience!
Thanks for the update @Mycobee
This is still in progress, I started moving forward with it today, and plan to work on it more tomorrow. Hopefully I will have a PR by the evening. I planned to have one earlier, but I ended up getting caught up with problems on another ticket I am working on.
So after grepping for 120 _REQUEST, _SUCCESS, and _FAILURE actions, I didn't have hits on any of them. Does that sound correct? I was ignoring node_modules/ and dist/ directories because I didn't think that should have any impact.
If this is correct, I may be able to pull out everything??
Below is my grep shell script.
declare -a ACTIONS=(
"GET_LOGS_REQUEST"
"GET_LOGS_SUCCESS"
"GET_LOGS_FAILURE"
"GET_AUDITS_REQUEST"
"GET_AUDITS_SUCCESS"
"GET_AUDITS_FAILURE"
"GET_CONFIG_REQUEST"
"GET_CONFIG_SUCCESS"
"GET_CONFIG_FAILURE"
"UPDATE_CONFIG_REQUEST"
"UPDATE_CONFIG_SUCCESS"
"UPDATE_CONFIG_FAILURE"
"RELOAD_CONFIG_REQUEST"
"RELOAD_CONFIG_SUCCESS"
"RELOAD_CONFIG_FAILURE"
"GET_ENVIRONMENT_CONFIG_REQUEST"
"GET_ENVIRONMENT_CONFIG_SUCCESS"
"GET_ENVIRONMENT_CONFIG_FAILURE"
"TEST_EMAIL_REQUEST"
"TEST_EMAIL_SUCCESS"
"TEST_EMAIL_FAILURE"
"TEST_SITE_URL_REQUEST"
"TEST_SITE_URL_SUCCESS"
"TEST_SITE_URL_FAILURE"
"TEST_S3_REQUEST"
"TEST_S3_SUCCESS"
"TEST_S3_FAILURE"
"INVALIDATE_CACHES_REQUEST"
"INVALIDATE_CACHES_SUCCESS"
"INVALIDATE_CACHES_FAILURE"
"RECYCLE_DATABASE_REQUEST"
"RECYCLE_DATABASE_SUCCESS"
"RECYCLE_DATABASE_FAILURE"
"CREATE_COMPLIANCE_REQUEST"
"CREATE_COMPLIANCE_SUCCESS"
"CREATE_COMPLIANCE_FAILURE"
"GET_COMPLIANCE_REQUEST"
"GET_COMPLIANCE_SUCCESS"
"GET_COMPLIANCE_FAILURE"
"UPLOAD_BRAND_IMAGE_REQUEST"
"UPLOAD_BRAND_IMAGE_SUCCESS"
"UPLOAD_BRAND_IMAGE_FAILURE"
"DELETE_BRAND_IMAGE_REQUEST"
"DELETE_BRAND_IMAGE_SUCCESS"
"DELETE_BRAND_IMAGE_FAILURE"
"GET_CLUSTER_STATUS_REQUEST"
"GET_CLUSTER_STATUS_SUCCESS"
"GET_CLUSTER_STATUS_FAILURE"
"TEST_LDAP_REQUEST"
"TEST_LDAP_SUCCESS"
"TEST_LDAP_FAILURE"
"SYNC_LDAP_REQUEST"
"SYNC_LDAP_SUCCESS"
"SYNC_LDAP_FAILURE"
"GET_LDAP_GROUPS_REQUEST"
"GET_LDAP_GROUPS_SUCCESS"
"GET_LDAP_GROUPS_FAILURE"
"LINK_LDAP_GROUP_REQUEST"
"LINK_LDAP_GROUP_SUCCESS"
"LINK_LDAP_GROUP_FAILURE"
"UNLINK_LDAP_GROUP_REQUEST"
"UNLINK_LDAP_GROUP_SUCCESS"
"UNLINK_LDAP_GROUP_FAILURE"
"SAML_CERT_STATUS_REQUEST"
"SAML_CERT_STATUS_SUCCESS"
"SAML_CERT_STATUS_FAILURE"
"UPLOAD_SAML_PUBLIC_REQUEST"
"UPLOAD_SAML_PUBLIC_SUCCESS"
"UPLOAD_SAML_PUBLIC_FAILURE"
"UPLOAD_SAML_PRIVATE_REQUEST"
"UPLOAD_SAML_PRIVATE_SUCCESS"
"UPLOAD_SAML_PRIVATE_FAILURE"
"UPLOAD_SAML_IDP_REQUEST"
"UPLOAD_SAML_IDP_SUCCESS"
"UPLOAD_SAML_IDP_FAILURE"
"DELETE_SAML_PUBLIC_REQUEST"
"DELETE_SAML_PUBLIC_SUCCESS"
"DELETE_SAML_PUBLIC_FAILURE"
"DELETE_SAML_PRIVATE_REQUEST"
"DELETE_SAML_PRIVATE_SUCCESS"
"DELETE_SAML_PRIVATE_FAILURE"
"DELETE_SAML_IDP_REQUEST"
"DELETE_SAML_IDP_SUCCESS"
"DELETE_SAML_IDP_FAILURE"
"TEST_ELASTICSEARCH_REQUEST"
"TEST_ELASTICSEARCH_SUCCESS"
"TEST_ELASTICSEARCH_FAILURE"
"PURGE_ELASTICSEARCH_INDEXES_REQUEST"
"PURGE_ELASTICSEARCH_INDEXES_SUCCESS"
"PURGE_ELASTICSEARCH_INDEXES_FAILURE"
"UPLOAD_LICENSE_REQUEST"
"UPLOAD_LICENSE_SUCCESS"
"UPLOAD_LICENSE_FAILURE"
"REMOVE_LICENSE_REQUEST"
"REMOVE_LICENSE_SUCCESS"
"REMOVE_LICENSE_FAILURE"
"GET_ANALYTICS_REQUEST"
"GET_ANALYTICS_SUCCESS"
"GET_ANALYTICS_FAILURE"
"UPLOAD_PLUGIN_REQUEST"
"UPLOAD_PLUGIN_SUCCESS"
"UPLOAD_PLUGIN_FAILURE"
"INSTALL_PLUGIN_FROM_URL_REQUEST"
"INSTALL_PLUGIN_FROM_URL_SUCCESS"
"INSTALL_PLUGIN_FROM_URL_FAILURE"
"GET_PLUGIN_REQUEST"
"GET_PLUGIN_SUCCESS"
"GET_PLUGIN_FAILURE"
"GET_PLUGIN_STATUSES_REQUEST"
"GET_PLUGIN_STATUSES_SUCCESS"
"GET_PLUGIN_STATUSES_FAILURE"
"REMOVE_PLUGIN_REQUEST"
"REMOVE_PLUGIN_SUCCESS"
"REMOVE_PLUGIN_FAILURE"
"ENABLE_PLUGIN_REQUEST"
"ENABLE_PLUGIN_SUCCESS"
"ENABLE_PLUGIN_FAILURE"
"DISABLE_PLUGIN_REQUEST"
"DISABLE_PLUGIN_SUCCESS"
"DISABLE_PLUGIN_FAILURE"
)
for ACTION in ${ACTIONS[@]}
do
echo $ACTION
grep -rnw --exclude-dir={node_modules,dist} mattermost-webapp/ -e $ACTION
grep -rnw --exclude-dir=node_modules mattermost-mobile/ -e $ACTION
done
Hi @hanzei , I don't mean to bother, but can you confirm that this looks correct? If so I am wondering if it is possible to remove the admin.ts file.
Thank you
@sudheerDev Can you help with this question?
@Mycobee Unfortunately that does not work all that well. I cross checked for all values to be sure and i think in this case we do use createCompliance in webapp at https://github.com/mattermost/mattermost-webapp/blob/dc33cec5eeb04cf721efd498a5a1ce998b3cc179/components/admin_console/compliance_reports/index.js#L40. For this entity we can get the error value in the component from the action https://github.com/mattermost/mattermost-webapp/blob/dc33cec5eeb04cf721efd498a5a1ce998b3cc179/components/admin_console/compliance_reports/compliance_reports.jsx#L90 eliminating the need to store error in redux state.
so after making the webapp change we can remove the reducer file and make other changes to actions to not dispatch the request flags.
let me know if you have any questions about this.
Hey @Mycobee, let us know if the above was helpful? If not, we'd be happy to clarify it :)
Hi,
Sorry for the delays...between work and life, I do not have the time to finish this pull request. Whoever picks it up, if they want to tag me, I have a lot of helpful PMs from @sudheerDev on the issue.
Hey @Mycobee totally understand, thanks for the heads up. Appreciate looking into it and hope to see you around in the community as time permits :)