Skip to content

Commit

Permalink
regression: remove query field on settings public listing (#33787)
Browse files Browse the repository at this point in the history
  • Loading branch information
ricardogarim authored Oct 28, 2024
1 parent af361a3 commit 1bdfd20
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 6 deletions.
6 changes: 6 additions & 0 deletions .changeset/shaggy-timers-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/rest-typings': major
'@rocket.chat/meteor': major
---

Changes settings public listing endpoint by moving query params from the 'query' attribute to standard query parameters.
13 changes: 11 additions & 2 deletions apps/meteor/app/api/server/v1/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import type {
} from '@rocket.chat/core-typings';
import { isSettingAction, isSettingColor } from '@rocket.chat/core-typings';
import { LoginServiceConfiguration as LoginServiceConfigurationModel, Settings } from '@rocket.chat/models';
import { isSettingsUpdatePropDefault, isSettingsUpdatePropsActions, isSettingsUpdatePropsColor } from '@rocket.chat/rest-typings';
import {
isSettingsUpdatePropDefault,
isSettingsUpdatePropsActions,
isSettingsUpdatePropsColor,
isSettingsPublicWithPaginationProps,
} from '@rocket.chat/rest-typings';
import { Meteor } from 'meteor/meteor';
import type { FindOptions } from 'mongodb';
import _ from 'underscore';
Expand Down Expand Up @@ -44,14 +49,18 @@ async function fetchSettings(
// settings endpoints
API.v1.addRoute(
'settings.public',
{ authRequired: false },
{ authRequired: false, validateParams: isSettingsPublicWithPaginationProps },
{
async get() {
const { offset, count } = await getPaginationItems(this.queryParams);
const { sort, fields, query } = await this.parseJsonQuery();
const { _id } = this.queryParams;

const parsedQueryId = typeof _id === 'string' && _id ? { _id: { $in: _id.split(',').map((id) => id.trim()) } } : {};

const ourQuery = {
...query,
...parsedQueryId,
hidden: { $ne: true },
public: true,
};
Expand Down
53 changes: 50 additions & 3 deletions apps/meteor/tests/end-to-end/api/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,56 @@ describe('[Settings]', () => {
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('settings');
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('success').and.to.be.true;
expect(res.body).to.have.property('settings').and.to.be.an('array').and.to.have.lengthOf(5);
expect(res.body).to.have.property('count').and.to.be.a('number').and.to.equal(5);
})
.end(done);
});
it('should return public settings even requested with _id param', (done) => {
void request
.get(api('settings.public'))
.query({
_id: 'Site_Url',
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success').and.to.be.true;
expect(res.body).to.have.property('settings').and.to.be.an('array').and.to.have.lengthOf(1);
expect(res.body).to.have.property('count').and.to.be.a('number').and.to.equal(1);
})
.end(done);
});
it('should return public settings even requested with _id param as an array', (done) => {
void request
.get(api('settings.public'))
.query({
_id: 'Site_Url,LDAP_Enable',
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success').and.to.be.true;
expect(res.body).to.have.property('settings').and.to.be.an('array').and.to.have.lengthOf(2);
expect(res.body).to.have.property('count').and.to.be.a('number').and.to.equal(2);
})
.end(done);
});
it('should return an empty response when requesting public settings with a broken _id param', (done) => {
void request
.get(api('settings.public'))
.query({
_id: 10,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success').and.to.be.true;
expect(res.body).to.have.property('settings').and.to.be.an('array').and.to.be.empty;
expect(res.body).to.have.property('count').and.to.be.a('number').and.to.equal(0);
expect(res.body).to.have.property('offset').and.to.be.a('number').and.to.equal(0);
expect(res.body).to.have.property('total').and.to.be.a('number').and.to.equal(0);
})
.end(done);
});
Expand Down
34 changes: 33 additions & 1 deletion packages/rest-typings/src/v1/settings.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { ISetting, ISettingColor, LoginServiceConfiguration } from '@rocket.chat/core-typings';

import type { PaginatedRequest } from '../helpers/PaginatedRequest';
import type { PaginatedResult } from '../helpers/PaginatedResult';
import { ajv } from './Ajv';

type SettingsUpdateProps = SettingsUpdatePropDefault | SettingsUpdatePropsActions | SettingsUpdatePropsColor;

Expand All @@ -25,9 +27,39 @@ type SettingsUpdatePropDefault = {

export const isSettingsUpdatePropDefault = (props: Partial<SettingsUpdateProps>): props is SettingsUpdatePropDefault => 'value' in props;

type SettingsPublicWithPaginationProps = PaginatedRequest<{ _id?: string; query?: string }>;

const SettingsPublicWithPaginationSchema = {
type: 'object',
properties: {
count: {
type: 'number',
nullable: true,
},
offset: {
type: 'number',
nullable: true,
},
sort: {
type: 'string',
nullable: true,
},
_id: {
type: 'string',
},
query: {
type: 'string',
},
},
required: [],
additionalProperties: false,
};

export const isSettingsPublicWithPaginationProps = ajv.compile<SettingsPublicWithPaginationProps>(SettingsPublicWithPaginationSchema);

export type SettingsEndpoints = {
'/v1/settings.public': {
GET: () => PaginatedResult & {
GET: (params: SettingsPublicWithPaginationProps) => PaginatedResult & {
settings: Array<ISetting>;
};
};
Expand Down

0 comments on commit 1bdfd20

Please sign in to comment.