openstreetmap-website
openstreetmap-website copied to clipboard
Redaction title can be blank.
@1v0ryh4t and I found that when creating a new redaction under /redactions/new
, the redaction title is allowed to be blank. When attempting to view said redactions under /redactions
, the new redaction is not present or viewable under the list as its title or name is an empty string. I would like to add a validation to a redaction
so that when creating or editing the redaction
, the name can never be blank. (This seems like a decent good-first-issue
for myself.)
Please see the attached images.
All redactions (/redactions
):
I think it's reasonable to require a title, should we also consider making the description required too?
I checked the original pull request for adding redactions, and I couldn't find any discussion as to whether titles and/or descriptions should be optional. I'd be interested in feedback from any openstreetmap.org moderators, who have the most experience with creating redactions. Personally I think it would be fine to require both attributes to be present.
I think it's clear a title should be required and I'd be inclined to say a description should be as well.
I'm surprised I didn't insist on it at the time - it must have slipped by me when I was reviewing this.
Does this still need to be worked on?
I'd be interested in feedback from any openstreetmap.org moderators
Creating new redactions isn't something that we need to do that often, thankfully -the last one I did was a couple of years ago I think - https://www.openstreetmap.org/redactions/102 . I'd always expect to use a title and description.
is this issue open can i work on it
is this issue open can i work on it
Yes, the issue is open and can be worked on. Please also read through the previous pull request for more information.
@BLSowmya Yes, the issue is open and you are welcome to work on it.
Hey @gravitystorm @tomhughes If this issue has not been picked up by anyone, I'm going to work on it, after reading the previous PR comments I want confirmation about some things:
- We want ensure the presence of Title and Description both,
- For missing titles and description we need fill them up with the id of the redaction?
- In a single migration we have to backfill our data and change the column null to false right?
- Since we are using strong migrations do we need to change null first and then Validate it inside a seperate migration as stated by the strong migration documentation?
We want ensure the presence of Title and Description both,
Yes please
For missing titles and description we need fill them up with the id of the redaction?
I would suggest "Redaction #{id}" (i.e. the word "Redaction" followed by the id number) as the title, and "No description" as the description. But it's not super important since I don't think there are many (any?) on the osm.org servers. It might affect other deployments though.
In a single migration we have to backfill our data and change the column null to false right? Since we are using strong migrations do we need to change null first and then Validate it inside a seperate migration as stated by the strong migration documentation?
Yes, whichever approach works best with strong migrations. See https://github.com/ankane/strong_migrations?tab=readme-ov-file#setting-not-null-on-an-existing-column
Thanks for looking into this!
@gravitystorm Thanks for the clarification's I'll start working on it
Model validations were added in 873ac155cabfb1aab8d887d07bc8389617d3f843 without realising that they fixed this.