-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: AJV schema for removing an agent from a department #36258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: dc9afaa The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-7.8.0 #36258 +/- ##
==============================================
Coverage 64.56% 64.56%
==============================================
Files 3147 3147
Lines 104614 104614
Branches 19768 19782 +14
==============================================
+ Hits 67544 67549 +5
+ Misses 34385 34379 -6
- Partials 2685 2686 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if only 2 fields are needed, we should not accept others, we should stop sending in the UI
|
Totally agree. However the endpoint accepted them before the ajv validator so removing them was a breaking change. I am writing a task to remove them on v8 |
|
btw not fix the frontend already?
|
|
Yup, backport is planned. Is one of the items I mentioned for 7.7x patch 😬 and ideally it should be on 7.8 but I just noticed the candidate is out already. Gonna check on Monday this. On the fe, since it was meant to be backported, I tried to not include more things. |
|
so a good mid ground solution is to allow the schema in the ajv, but remove it from the type and fix it in the frontend. Also, open a PR removing it from version 8.0.0 already When you do the backport, you can leave out the frontend if (you want) and type and just bring the schema (as was your first suggestion), but to consider this done all the mention points should be solved |
78eac36 to
f7a4329
Compare
|
@ggazzo you can check it now :) I moved this one to 7.8 branch as we have to patch it to 7.7 anyways |
|
/patch |
|
Pull request #36298 added to Project: "Patch 7.7.2" |
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/CTZ-230
https://rocketchat.atlassian.net/browse/CTZ-233
Steps to test or reproduce
Further comments
Not so common to remove agents this way, but it was not working. Caused by a refactor on 7.7.
Introduced by: #35854