Feature flags 717

[popkinj]

New toggle attribute within the local.json and default.json files to turn all writable features (aside from contacts) on/off.

Here is the full list of features effected:

• Remove function -> Copy Schema
• Remove function -> Create Schema
• Remove function -> Create a Message from messages page
• Remove function -> Create a Message from message sidebar in the connections page
• Remove function -> Edit and Delete Contact
• Remove function -> Add/Update OCA Associations
• Remove function -> All buttons in the CredentialsTable.vue
• Remove function -> Verifications; Create Presentation Request, Delete and Copy
• Remove function -> Create Credential Definition
• Remove function -> Create Presentation Request... and delete and copy in the table

[github-actions]

Deployment	Resource	Location
Traction
	Tenant UI	https://pr-743-tenant-ui-dev.apps.silver.devops.gov.bc.ca
	Innkeeper UI	https://pr-743-tenant-ui-dev.apps.silver.devops.gov.bc.ca/innkeeper
Aca-Py
	Admin Swagger	https://pr-743-traction-acapy-admin-dev.apps.silver.devops.gov.bc.ca/api/doc
	Tenant Proxy Swagger	https://pr-743-traction-tenant-proxy-dev.apps.silver.devops.gov.bc.ca/api/doc

Deployment URLs ready for review.

[popkinj]

I'm seeing some build errors.... Assuming it has something to do with the Helm chart still needing an update. Will look into that now.

[loneil]

default.json is not sufficient to control deployment values. That is just the hard-coded built setting if the environmental setting is not there.
Need to add same key to custom-environment-variables.json (uses standard Node-Config hierarchy) and add an environment variable name that can be set by the Helm Charts.

For example, see how in

• default.json there is the default value for showInnkeeperAdminLogin
• And in custom-environment-variables.json that maps it to an env var showInnkeeperAdminLogin": "FRONTEND_INNKEEPER_SHOW_ADMIN",
• Then to get that variable set from helm, in config.yaml it sets up a helm chart mapping
  FRONTEND_INNKEEPER_SHOW_ADMIN: {{ .Values.oidc.showInnkeeperAdminLogin | quote }}
• which is then specified per-environment in the corresponding values files, such as values-pr.yaml, values-production.yaml etc.

Same thing should be done here with the new showWritableComponents

Build errors aren't related to Helm values, it's Unit Tests failing. Check reason for failures with "details" link in PR

(or run npm run test locally in the frontend folder)
Someone else from Quartech PR'd a lot of unit test coverage so I'd assume changes like taking away buttons could trigger failures (though default value being true here I'm not sure how would affect UTs, might need mocks or something).
I haven't had a chance to dig much into the unit test snapshot stuff with ViteTest. Akiff knows more and may be able to help, or skip specific tests if they are getting in the way too much for now.

[jamshale]

The unit tests are failing because I wrote some tests that was preventing features that I thought were important from being removed. They are basically doing what they are supposed to.

To fix them:

• I removed the delete connections tests in /components/connections/Connections.test.ts
• The OfferCredential component check in issuedCredentials component because it's hidden now.
• The CreateMessage component check in Messages component.
• The CreateRequest and DeleteExchangeRecord components check in Presentations.

I could fix it for you but haven't got direct push access to the repo yet. I still need to do a fork.

I know it's a bit of a change but having the tests run when you are making changes is a good way to do continuous testing. These tests would have failed when you saved the file.
Maybe the tests were a bit to intrusive. But i think they were valuable if someone hid these components by mistake.

[popkinj]

The unit tests are failing because I wrote some tests that was preventing features that I thought were important from being removed. They are basically doing what they are supposed to.

To fix them:

• I removed the delete connections tests in /components/connections/Connections.test.ts
• The OfferCredential component check in issuedCredentials component because it's hidden now.
• The CreateMessage component check in Messages component.
• The CreateRequest and DeleteExchangeRecord components check in Presentations.

I could fix it for you but haven't got direct push access to the repo yet. I still need to do a fork.

I know it's a bit of a change but having the tests run when you are making changes is a good way to do continuous testing. These tests would have failed when you saved the file. Maybe the tests were a bit to intrusive. But i think they were valuable if someone hid these components by mistake.

All good @jamshale ... At least we know they're doing what they're supposed to.
I'll have a look at doing the test updates in my branch. When done I'll let you know so you can have a look. :)

[esune]

@popkinj @jamshale if the end to end test can be aware of the flag and check things accordingly it would be the best of both worlds.

[jamshale]

@popkinj @jamshale if the end to end test can be aware of the flag and check things accordingly it would be the best of both worlds.

@esune You're right. The better way than removing the tests is to set the showWritableComponents flag in configStore mock in /test/__mocks__/store/config.ts to true. There are other flags there. Because this is undefined is why the UI components aren't showing.

You could toggle it to false during a test and make sure they are hidden to test this feature.

There is an example toggling the config store in the last test in Login.test.ts

[jamshale]

I don't have a problem with removing the tests but i think you should declare the flag as false in /test/__mocks__/store/config.ts the config store mock instead of leaving it undefined.

[loneil]

Yeah I'd rather see tests for the on/off cases for this flag (and agree, declare it in the mock test data) but if removing them can get this ticket done then can move forward and address later.

[popkinj]

I don't have a problem with removing the tests but i think you should declare the flag as false in /test/__mocks__/store/config.ts the config store mock instead of leaving it undefined.

Done. 👍

[popkinj]

Looks like this is working now. 🎉

[loneil]

Looks good with the false flag working now. Couple minor points:

Think there's a spot or 2 missing the hiding

• IssuedCredentials.vue, the DeleteCredentialExchangeButton and RevokeCredentialButton (referenced in issue, revoke is at least)
• Schema Storage Delete Schema button.

Verifications page seems to pull the search over the left when the button is gone

Issuance page behaves correct here so can just copy what that is doing

[loneil]

Looks good to me 👍
Tried flipping the config val manually in OCP console and looks good in both modes now.
Can just flip that flag in values-pr back to true now that testing is done and then can merge.