Uli comm backend #660
Uli comm backend #660
Conversation
maanasb01
force-pushed
the
uli-comm-backend
branch
from
e403df1 to
123b207
Compare
| add :appropriated, :boolean, default: false, null: false | ||
| add :appropriation_context, :boolean, default: false, null: false | ||
| add :meaning, :text | ||
| add :category, {:array, :string} |
There was a problem hiding this comment.
is there a way to make it array of enum values?
There was a problem hiding this comment.
Hey, I think it is possible, I would need to check but I think it can be done. But my doubt here is that we were letting the user add categories from the UI where we are providing the options in a dropdown. So I was thinking about whether we should keep controlling the user input from the UI itself.
We can maintain this enum in the database and then fetch these categories for options to be provided in the UI. But this might be not the very ideal case given that the categories are going to be mostly unchanged, and also doing this would make the frontend make a request every time the form comes up which could be unnecessary given the mentioned reason.
We can also maintain the categories in 2 places, one in the database and the other in the UI, but we would need to sync them in case there is any change in the categories.
For these reasons, I thought to control the categories from the UI only. Please let me know what your thoughts on this.
There was a problem hiding this comment.
As you correctly said, the categories wont be changed often, so it doesnt justify making an API call for them. We'll maintain the category list in two places - backend and frontend. we'll have to remember to update at both places when changes need to be made.
My reason to insist on using enums for this is that it would be more egregious if as an error we save a unrecognized category value. So defining this as an enum would protect against it.
There was a problem hiding this comment.
Cool, so will make it an array of enums them. Will push the changes here.
| end | ||
|
|
||
| @doc false | ||
| def changeset(crowdsourced_slur, attrs) do |
There was a problem hiding this comment.
A tip. There's a few ways to create crowdsouced_slur
- Only add the label and none of the other fields
- Add label and one or more of the other fields
- Add/update values like category, level_of_severity etc for an already existing slur
You might need more than one changeset to capture these possibilities.
Conventionally these functions should be named changeset_*. We can discuss appropriate names later.
There was a problem hiding this comment.
Hey, yeah sure I will add more methods to create an entry with different scenarios. This was generated by the Phoenix generator. I was planning to add these on a requirement basis when I integrate the FE and BE.
Regarding this, just want to confirm, when I see the "add slur" form, I see that apart from appropriation_context and "what makes it problematic" fields, all other fields are necessary, so should I only create the changeset regarding these?
|
@maanasb01 I see that these three stories are getting addressed now : I have a feeling that #652 would also work but since I cant save my preferences, i am not able to test this. Regardless, can i link the first three issues to this PR and merge this into dev? |
|
Hey Denny, I have just pushed all the changes, including adding the signup redirect on the login page. Regarding those sub-issues, yes they are all resolved now. For issue #652, are you referring to the "Enable Slur Metadata" option? If yes, then it is also resolved. Users can enable both options replacement/show-metadata without any authentication. One more thing, with the implementation of these new changes, the crowd-sourced slur feature is broken. I will fix it next while working on the slurs functionality, but just letting you know. If you want, I can change it such that the option to "add slur" would only come if the user is logged in, in this PR itself. Otherwise, would make all the changes in the new PR. Let me know your thoughts! |
|
@maanasb01 All sounds good. |
This is a WIP PR.
The following tasks were covered in this PR:
CC: @aatmanvaidya