Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

changed user props field type #718

Closed
wants to merge 1 commit into from
Closed

changed user props field type #718

wants to merge 1 commit into from

Conversation

1kjnv
Copy link

@1kjnv 1kjnv commented Aug 24, 2022

Summary

Changed User Props field type from "object" to map[string]string.

Ticket Link

Fixes #643

@mattermod
Copy link
Contributor

Hello @ikjnv,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

@1kjnv 1kjnv marked this pull request as ready for review August 24, 2022 08:36
@1kjnv
Copy link
Author

1kjnv commented Aug 24, 2022

/check-cla

@cwarnermm cwarnermm self-requested a review August 24, 2022 13:10
@plant99 plant99 self-requested a review August 24, 2022 13:18
@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@plant99
Copy link
Contributor

plant99 commented Sep 6, 2022

Hi @ikjnv , @cwarnermm apologies for the delay.

I was thinking about this PR offline, and realized that mattermost-server doesn't maintain a schema for this.

So, props like notify_props can take a much broader object than the two fields mentioned here. It'd take some time for me to collect all of them.

This PR looks very good to me, but would it mislead the users regarding extent of user.props?

@cwarnermm
Copy link
Member

Thanks, @plant99, for reviewing this PR. @laneycs - Can a member of the Suite Users Team review and advise on next steps for this PR? I'm specifically looking for guidance on whether the content update proposed in this PR is sufficient, or if there's better value to our users if we document the broader capabilities of these two objects. If it's the latter, I'll need some help from the team to document this information.

@laneycs
Copy link

laneycs commented Sep 6, 2022

@cwarnermm Sure! Will add to our triage backlog to review with the team.

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@cwarnermm cwarnermm added 2: Dev Review Requires review by a core committer and removed Lifecycle/1:stale labels Sep 30, 2022
@cwarnermm cwarnermm requested a review from laneycs September 30, 2022 14:04
@cwarnermm
Copy link
Member

@laneycs - Does your team have an ETA on when this PR will be technically reviewed?

@laneycs
Copy link

laneycs commented Oct 4, 2022

@cwarnermm Looks like our team still needs to triage this ticket. You can follow its progress here: https://mattermost.atlassian.net/browse/MM-46897. We triage incoming items in our backlog every Tuesday and try to make room for dedicated maintenance and tech debt work 1 day per week. Hoping we're able to triage and take action on it in the next couple of weeks.

Heads up that some of our maintenance work has been pushed back as we have instructions from MLT to develop packaging changes for the 7.6 release, so we've had to make a bit of a pivot in our roadmap.

@cwarnermm
Copy link
Member

Thank you, @laneycs! Really appreciate the context and link to the Jira ticket driving this effort.

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@mattermost-build
Copy link

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2: Dev Review Requires review by a core committer Contributor Lifecycle/1:stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Help Wanted: User props is not fully described in API reference
6 participants