feat(condo): DOMA-6736 schema MobileFeatureConfig with tests #3662

VKislov · 2023-07-26T09:32:50Z

No description provided.

AleX83Xpert

Is this model will be used only for disabling the ability to send tickets?
Is there a new domain needed? What about using one of the existing domains (organization, user, resident, ticket)?
Also, please fix the code according to the linter rules.

apps/condo/domains/settings/constants.js

apps/condo/domains/settings/utils/clientSchema/MobileFeatureConfig.ts

apps/condo/domains/settings/access/MobileFeatureConfig.js

apps/condo/domains/settings/schema/MobileFeatureConfig.js

apps/condo/domains/settings/access/MobileFeatureConfig.js

pahaz · 2023-08-01T16:41:37Z

apps/condo/domains/settings/schema/MobileFeatureConfig.js

+}
+
+const MobileFeatureConfig = new GQLListSchema('MobileFeatureConfig', {
+    schemaDoc: 'Manages availability of some features in mobile application,' +


It’s unclear. It assumes that you have one model per one feature.

MobileFeaturesConfig?

but google translate says that MobileFeatureConfig is ok)

I think @pahaz means that you will not be able to customize other features using this model. See my questions too.

pahaz · 2023-08-01T16:43:52Z

apps/condo/domains/settings/schema/MobileFeatureConfig.js

+            type: Text,
+        },
+
+        ticketSubmittingIsDisabled: {


Is ticket submission disabled
or is tickets disabled.

Naming the fields like that, I proceeded from a different logic. Now there are 5 fields needed to configure 2 features. When there are 30 fields, finding the right one will be a difficult task both on the client and on the server. Therefore, I write the feature prefix and then the field itself. That is, the feature ticketSubmitting and then what the field stores. If you change ticketSubmittingIsEnabled to ticketSubmission, then searching for all fields related to the ticketSubmitting feature will be impossible and you will have to manually search for everything.

What about using the same prefix for auxiliary field naming?

pahaz · 2023-08-01T16:44:51Z

apps/condo/domains/settings/schema/MobileFeatureConfig.js

+            },
+        },
+
+        onlyProgressionMeterReadingsIsEnabled: {


How to be with meter counter overflow?

Didn't understand what you mean. Haven't heard of counter overflow

In this case, the resident will call the organization and they will manually enter the testimony through CRM, after which the resident will be able to submit again

pahaz · 2023-08-01T16:46:54Z

apps/condo/domains/settings/schema/MobileFeatureConfig.js

+            },
+        },
+
+        onlyProgressionMeterReadingsIsEnabled: {


Is allow only greater then previous meter reading submission enabled :)
(Like others Boolean flags)

apps/condo/domains/settings/schema/MobileFeatureConfig.js

Alllex202 · 2023-08-02T04:37:23Z

pls add description for your pr

apps/condo/domains/settings/components/ticketSubmitting/OnlyProgressionMeterReadingsForm.tsx

...ndo/domains/settings/components/ticketSubmitting/OnlyProgressionMeterReadingsSettingCard.tsx

apps/condo/domains/settings/components/ticketSubmitting/TicketSubmittingSettingCard.tsx

apps/condo/domains/settings/schema/MobileFeatureConfig.test.js

apps/condo/pages/settings/mobileFeatureConfig/onlyProgressionMeterReadings/index.tsx

apps/condo/pages/settings/mobileFeatureConfig/ticketSubmitting/index.tsx

VKislov · 2023-08-02T07:55:23Z

pls add description for your pr

The title has a task. You need to open it task and there is all the context / links / pictures. I think writing the same thing in 100 places is a waste of resources

apps/condo/domains/organization/gql.js

apps/condo/domains/settings/access/MobileFeatureConfig.js

apps/condo/domains/settings/components/ticketSubmitting/OnlyProgressionMeterReadingsForm.tsx

apps/condo/domains/settings/schema/MobileFeatureConfig.js

AleX83Xpert · 2023-08-07T04:33:15Z

apps/condo/domains/settings/access/MobileFeatureConfig.js

+    return {
+        organization: {
+            OR: [
+                queryOrganizationEmployeeFor(user.id),
+                queryOrganizationEmployeeFromRelatedOrganizationFor(user.id),
+            ],
+        },
+    }


Are every employee can read this model (technician staff)? Maybe canReadMobileFeatureConfigs should appear?

Yes, and I think that's right. The organization settings page is available to everyone at the moment. Special rights are only used to check editing rights. I think it will be correct to repeat the behavior of the other sections of the organization settings

AleX83Xpert · 2023-08-07T06:27:25Z

apps/condo/domains/settings/schema/MobileFeatureConfig.js

+const MobileFeatureConfig = new GQLListSchema('MobileFeatureConfig', {
+    schemaDoc: 'Manages availability of some features in mobile application,' +
+        ' stores data required in disabled state of a feature.',
+    fields: {


It's not understandable how auxiliary fields belong to feature (enabled/disabled) areas.

This data is necessary for the mobile application to determine what to display. In the case of the feature of disabling tickets - displaying the desired phone.
If you are describing the full changes that will occur in the mobile app when you disable/enable a feature, it will mean that this place must be maintained when changes are made in the mobile app, otherwise this data source will mislead the backend developer.

What about using the same prefix for auxiliary field naming?

that's how it happens. All fields required for feature management are marked with a prefix. That is why the ticketSubmittingIsDisabled field is called so incorrectly (from the point of view of the English language). Pahaz asked for some fields, for example, the org phone, which will be reused with a high probability, not to attach to a specific feature, but to leave commonPhone

AleX83Xpert · 2023-08-07T06:29:32Z

apps/condo/domains/settings/schema/MobileFeatureConfig.js

+            type: Text,
+        },
+
+        ticketSubmittingIsDisabled: {


What about using the same prefix for auxiliary field naming?

AleX83Xpert · 2023-08-07T06:34:23Z

apps/condo/domains/settings/schema/MobileFeatureConfig.test.js

+    })
+    describe('CRUD tests', () => {
+        describe('accesses', () => {
+            describe('admin', () => {


If we decide in the future to restrict access for support users, the tests will not show any errors. For now, we have a separate type of user and have no test for them.

apps/condo/domains/settings/schema/MobileFeatureConfig.test.js

AleX83Xpert · 2023-08-07T06:44:05Z

apps/condo/pages/settings/index.tsx


    const hasSubscriptionFeature = hasFeature('subscription')
-
+    const { useFlag } = useFeatureFlags()
+    const hasMobileFeatureConfigurationFeature = useFlag(MOBILE_FEATURE_CONFIGURATION)


Looks like I still have the ability to create a model using graphql API directly.

this is true

AleX83Xpert · 2023-08-07T06:46:25Z

apps/condo/domains/common/components/settings/MobileFeatureConfigContent.tsx

+    const hasMobileFeatureConfigurationFeature = useFlag(MOBILE_FEATURE_CONFIGURATION)
+    const hasTicketSubmittingSettingFeature = useFlag(TICKET_SUBMITTING_SETTING)
+    const hasOnlyProgressionMeterReadingsSettingFeature = useFlag(ONLY_PROGRESSION_METER_READINGS_SETTING)


Is every feature controlled by two feature flags combination?

Yes, this is a business requirement. It is necessary that the org manages this itself. But at the same time, we do not want every user to see all the settings. Since we are developing them, not so that they would be turned off, but so that they would be used. This task is a rare case when it is necessary

apps/condo/domains/settings/constants.js

apps/condo/pages/settings/mobileFeatureConfig/onlyProgressionMeterReadings/index.tsx

apps/condo/pages/settings/mobileFeatureConfig/ticketSubmitting/index.tsx

apps/condo/domains/settings/components/ticketSubmitting/TicketSubmittingSettingsForm.tsx

apps/condo/domains/settings/components/ticketSubmitting/OnlyProgressionMeterReadingsForm.tsx

apps/condo/domains/settings/schema/MobileFeatureConfig.js

apps/condo/domains/common/components/settings/MobileFeatureConfigContent.tsx

apps/condo/domains/common/constants/featureflags.js

apps/condo/domains/settings/access/MobileFeatureConfig.js

apps/condo/domains/settings/utils/MobileFeatureConfigSchemaValidations.js

apps/condo/pages/settings/mobileFeatureConfig/onlyProgressionMeterReadings/index.tsx

apps/condo/pages/settings/mobileFeatureConfig/ticketSubmitting/index.tsx

apps/condo/domains/settings/schema/MobileFeatureConfig.js

apps/condo/domains/settings/utils/MobileFeatureConfigSchemaValidations.js

apps/condo/pages/settings/mobileFeatureConfig/onlyProgressionMeterReadings/index.tsx

apps/condo/pages/settings/mobileFeatureConfig/ticketSubmitting/index.tsx

…d to ticketSubmittingIsDisabled. Rm emergencyPhone field

… for manage access

… onlyGreaterThanPreviousMeterReadingIsEnabled

some optimization on front, rm redundant react state. add tests for support. move FF constants to common/constants/featureFlags. fix schemaDoc for meta

Alllex202 · 2023-08-11T13:24:51Z

Why don't you add this config automatically for all organizations? It's a 1 to 1 relationship, why not?

apps/condo/domains/common/components/settings/MobileFeatureConfigContent.tsx

Co-authored-by: Alllex202 <[email protected]>

* feat(condo): DOMA-6736 schema MobileFeatureConfig with tests * fix(condo): DOMA-6736 change field name from ticketSubmittingIsEnabled to ticketSubmittingIsDisabled. Rm emergencyPhone field * fix(condo): DOMA-6736 fix imports * feat(condo): DOMA-6753 frontend of MobileFeatureConfig * fix(condo): DOMA-6736 schemaDoc of some field of MobileFeatureConfig * fix(condo): DOMA-6736 review fixes * feat(condo): DOMA-6736 added permission canManageMobileFeatureConfigs for manage access * fix(condo): DOMA-6736 rename onlyProgressionMeterReadingsIsEnabled to onlyGreaterThanPreviousMeterReadingIsEnabled * fix(condo): DOMA-6736 replace catchErrorFrom to expectToThrowGQLError * fix(condo): DOMA-6736 use valuePropName * fix(condo): DOMA-6736 add deletedAt filter * fix(condo): DOMA-6736 review fixes some optimization on front, rm redundant react state. add tests for support. move FF constants to common/constants/featureFlags. fix schemaDoc for meta * fix(condo): DOMA-6736 some code improvements * chore(condo): DOMA-6736 fix imports Co-authored-by: Alllex202 <[email protected]> --------

pahaz · 2024-05-13T19:38:51Z

apps/condo/domains/settings/access/MobileFeatureConfig.js

+    if (user.isAdmin || user.isSupport) return {}
+
+    if (user.type === RESIDENT) {
+        const residents = await find('Resident', { user: { id: user.id }, deletedAt: null })


You need to check others models. We need to get organizations from ServiceConsumer

pahaz · 2024-05-13T19:48:59Z

apps/condo/domains/common/components/settings/MobileFeatureConfigContent.tsx

+
+const CONTENT_GUTTER: Gutter | [Gutter, Gutter] = [0, 40]
+
+export const MobileFeatureConfigContent: React.FC = () => {


Confusing naming. It's look like some guard or some wrapper for feature flag checks.

It will be better to create some wraper or HOC for this task. For example:

const CardsContainer = ... const CardsContainerWithAccessCheck = addFeatureFlagIsEnabled(CardsContainer, 'MOBILE_FEATURE')

VKislov added ✋🙂 Review please Comments are resolved, take a look, please 🚨 Migrations We have a database migrations here! labels Jul 26, 2023

AleX83Xpert reviewed Jul 26, 2023

View reviewed changes

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch 3 times, most recently from b965a29 to 5b384fa Compare July 27, 2023 10:42

AntonAL reviewed Jul 28, 2023

View reviewed changes

apps/condo/domains/settings/schema/MobileFeatureConfig.js Outdated Show resolved Hide resolved

apps/condo/domains/settings/schema/MobileFeatureConfig.js Outdated Show resolved Hide resolved

AntonAL reviewed Jul 28, 2023

View reviewed changes

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch from 5b384fa to 453132b Compare July 31, 2023 12:51

VKislov requested review from AntonAL and AleX83Xpert August 1, 2023 07:50

pahaz reviewed Aug 1, 2023

View reviewed changes

apps/condo/domains/settings/access/MobileFeatureConfig.js Outdated Show resolved Hide resolved

pahaz reviewed Aug 1, 2023

View reviewed changes

apps/condo/domains/settings/schema/MobileFeatureConfig.js Show resolved Hide resolved

Alllex202 reviewed Aug 2, 2023

View reviewed changes

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch 2 times, most recently from 3ddbe5b to c14a6fa Compare August 2, 2023 11:10

VKislov requested a review from Alllex202 August 2, 2023 11:11

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch from aaea781 to a0becf7 Compare August 2, 2023 11:38

AntonAL reviewed Aug 4, 2023

View reviewed changes

apps/condo/domains/organization/gql.js Show resolved Hide resolved

AntonAL reviewed Aug 4, 2023

View reviewed changes

apps/condo/domains/settings/access/MobileFeatureConfig.js Outdated Show resolved Hide resolved

AntonAL reviewed Aug 4, 2023

View reviewed changes

apps/condo/domains/settings/components/ticketSubmitting/OnlyProgressionMeterReadingsForm.tsx Outdated Show resolved Hide resolved

apps/condo/domains/settings/schema/MobileFeatureConfig.js Show resolved Hide resolved

AleX83Xpert reviewed Aug 7, 2023

View reviewed changes

Alllex202 reviewed Aug 7, 2023

View reviewed changes

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch from c8260a1 to 13da42e Compare August 8, 2023 10:37

VKislov requested a review from Alllex202 August 8, 2023 10:37

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch 2 times, most recently from 5ea4b04 to 9f8ff61 Compare August 9, 2023 05:01

Alllex202 reviewed Aug 9, 2023

View reviewed changes

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch from b578cbc to ce9c05f Compare August 10, 2023 09:43

Alllex202 reviewed Aug 11, 2023

View reviewed changes

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch from ce9c05f to 8b92716 Compare August 11, 2023 08:37

VKislov added 13 commits August 11, 2023 14:05

feat(condo): DOMA-6736 schema MobileFeatureConfig with tests

508dd80

fix(condo): DOMA-6736 change field name from ticketSubmittingIsEnable…

5337e40

…d to ticketSubmittingIsDisabled. Rm emergencyPhone field

fix(condo): DOMA-6736 fix imports

58f7c90

feat(condo): DOMA-6753 frontend of MobileFeatureConfig

30583bc

fix(condo): DOMA-6736 schemaDoc of some field of MobileFeatureConfig

f75703e

fix(condo): DOMA-6736 review fixes

62bd014

feat(condo): DOMA-6736 added permission canManageMobileFeatureConfigs…

c24ff34

… for manage access

fix(condo): DOMA-6736 rename onlyProgressionMeterReadingsIsEnabled to…

e73aceb

… onlyGreaterThanPreviousMeterReadingIsEnabled

fix(condo): DOMA-6736 replace catchErrorFrom to expectToThrowGQLError

79cc51d

fix(condo): DOMA-6736 use valuePropName

563baee

fix(condo): DOMA-6736 add deletedAt filter

9db39ad

fix(condo): DOMA-6736 review fixes

4b6b438

some optimization on front, rm redundant react state. add tests for support. move FF constants to common/constants/featureFlags. fix schemaDoc for meta

fix(condo): DOMA-6736 some code improvements

2ade841

VKislov force-pushed the feat/condo/DOMA-6736/mobile-feature-configurator branch from 8b92716 to 2ade841 Compare August 11, 2023 09:35

Alllex202 approved these changes Aug 11, 2023

View reviewed changes

apps/condo/domains/common/components/settings/MobileFeatureConfigContent.tsx Outdated Show resolved Hide resolved

chore(condo): DOMA-6736 fix imports

bbf8d93

Co-authored-by: Alllex202 <[email protected]>

VKislov merged commit c9eadcc into master Aug 12, 2023
7 checks passed

VKislov deleted the feat/condo/DOMA-6736/mobile-feature-configurator branch August 12, 2023 17:58

pahaz reviewed May 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(condo): DOMA-6736 schema MobileFeatureConfig with tests #3662

feat(condo): DOMA-6736 schema MobileFeatureConfig with tests #3662

VKislov commented Jul 26, 2023

AleX83Xpert left a comment

pahaz Aug 1, 2023 •

edited

Loading

VKislov Aug 2, 2023

VKislov Aug 2, 2023

AleX83Xpert Aug 4, 2023

pahaz Aug 1, 2023

VKislov Aug 2, 2023

AleX83Xpert Aug 7, 2023

pahaz Aug 1, 2023

VKislov Aug 2, 2023

VKislov Aug 7, 2023

pahaz Aug 1, 2023 •

edited

Loading

Alllex202 commented Aug 2, 2023

VKislov commented Aug 2, 2023

AleX83Xpert Aug 7, 2023

VKislov Aug 7, 2023

AleX83Xpert Aug 7, 2023

VKislov Aug 7, 2023

VKislov Aug 7, 2023

AleX83Xpert Aug 7, 2023

AleX83Xpert Aug 7, 2023

AleX83Xpert Aug 7, 2023

VKislov Aug 7, 2023

AleX83Xpert Aug 7, 2023

VKislov Aug 7, 2023

Alllex202 commented Aug 11, 2023

pahaz May 13, 2024 •

edited

Loading

pahaz May 13, 2024


		const CONTENT_GUTTER: Gutter \| [Gutter, Gutter] = [0, 40]

		export const MobileFeatureConfigContent: React.FC = () => {

feat(condo): DOMA-6736 schema MobileFeatureConfig with tests #3662

feat(condo): DOMA-6736 schema MobileFeatureConfig with tests #3662

Conversation

VKislov commented Jul 26, 2023

AleX83Xpert left a comment

Choose a reason for hiding this comment

pahaz Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pahaz Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

Alllex202 commented Aug 2, 2023

VKislov commented Aug 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alllex202 commented Aug 11, 2023

pahaz May 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pahaz Aug 1, 2023 •

edited

Loading

pahaz Aug 1, 2023 •

edited

Loading

pahaz May 13, 2024 •

edited

Loading