Project names cannot include upper-case letters

[knolleary]

Current Behavior

You cannot create a project with an upper-case letter in the name.

Whilst this makes some sense as they are used for DNS names, we should support upper-case letters in the name.

It isn't entirely trivial to just relax the regex. The database model constraint is currently used to ensure unique names - but that is a case-sensitive check. We would now need to have a case-insensitive check.

Expected Behavior

No response

Steps To Reproduce

No response

Environment

• FlowForge version:
• Node.js version:
• npm version:
• Platform/OS:
• Browser:

[Steve-Mcl]

Hi Nick, had a chat with @hardillb & several ideas were explored.

Would the following solution be acceptable?...

1. Do NOT add 2nd field

2. Permit mixed case to be written to name field of DB

3. Add a virtual field safeName that returns the lcase of the name field

    4. When determining if a name is already used (.count({ where: { name: name }})) (ref) is called, use the safeName field and the lowercase of request.body.name in the where clause. Alternatively, this could be abstracted to a function in the model (e.g. isNameAvailable(name))

4. Add an abstracted function to the model (i.e. isNameAvailable(name))

This permits us to allow mixed case names without modifying all the UI code (that currently accesses & displays) project.name

For DNS / slugs areas of the code, we can use project.safeName

Assuming this works and is acceptable, this solution would...

• avoid the need to add a DB migration & maintain 2 fields
• avoid the need to modify lots of UI code for choosing which field to display
• An additional benefit/future feature might be that the virtual field could be slugified permitting more variation in the name than a-zA-Z-] (i.e. spaces)

[ZJvandeWeg]

@Steve-Mcl this method does not allow you to push a unique constraint to the database, as the count query does not insert. Might be a downside worth considering

[Steve-Mcl]

@ZJvandeWeg not 100% sure I follow your meaning.

Are you saying the database should have a unique constraint on the name field (it currently does not). There is nothing in the proposal to stop us adding a unique constraint on the name field before or after this modification.

Also, are you saying the INSERT query should be permitted and fail on the UNIQUE constraint check instead of using business logic to test for the existence of the project name?

In an ideal world, a DB constraint is the right way to go however PostGRES doesnt have the ability to set a column to case insensitive so maintaining 2 fields is the only option.

As for sequelise to handle a case insensitive check on the column, the methods for SQLITE and Postgres are different...
SQLITE --> COLLATE NOCASE
Postgres --> ILIKE

I am open to further suggestions or to implement a 2nd column - if this is the preferred method for reasons I have not considered, please let me know asap.

[knolleary]

safeName cannot be a virtual field - it needs to be an actual column in the database with a unique constraint on.

If it was a virtual field, we'd have no efficient way to check the unique constraint - sequelize would have to retrieve every single record in the database and then programmatically search through the virtual fields. That is work the database is optimised to do and we should use it.

To introduce this new column will require a migration - plenty of examples now to base that on.

At this stage, given we don't support changing the project name, we only need to ensure the field gets populated when the project is created.

[Steve-Mcl]

NP. I will re-do the PR to include a DB migration and additional field.

[Steve-Mcl]

Hi Nick @knolleary

I have revised this to add a new field safeName (non virtual).

The DB migration first adds the column without constraints (as it would fail), then copies the lower case value from name then applies the required constraints.

The tests were updated to ensure create project fails with different case/same name, and passes with a unique uppercase name, and that the title gets the value of the project name as per users entry.

TODO
What I suspect is outstanding on this is something I am unfamiliar with in the code base - DNS name generation and other places where the safeName should be used instead of the entered name. Can you or @hardillb point me in the right direction (if required) please?

[Steve-Mcl]

Testing on stage ok