Fix photo atom state bug #1141
Fix photo atom state bug #1141
Conversation
| @@ -29,6 +29,13 @@ defmodule AniminaWeb.StoriesComponents do | |||
| language={@language} | |||
| delete_story_modal_text={@delete_story_modal_text} | |||
| user={@user} | |||
| state={ | |||
There was a problem hiding this comment.
photo.state is not set by some magic in the background which we can not control. If there is a bug somewhere in our code which sets this state in a wrong way we have to fix THAT bug and not add more code afterwards to "fix" it. This shouldn't happen in the first place. Please find the original bug!
There was a problem hiding this comment.
And (of course!) after fixing the original bug we have to make sure that already existing database entries are fixed too (e.g. with a migration).
There was a problem hiding this comment.
I thought about this , in the database with IEX , all come as an atom , also we have it in the validation
attribute :state, :atom do
constraints one_of: [:pending_review, :in_review, :approved, :rejected, :error, :nsfw]
So this must be an ash bug or something similar where it displays atoms in the database as strings.
Although when you have an atom for example :stefan in the HTML , it is displayed as "stefan"
There was a problem hiding this comment.
- Is there code somewhere which writes into the database while bypassing the validation?
- If this is an Ash bug: Did you ask in the elixirforum.com about it and/or created an issue in the Ash GitHub repo?
There was a problem hiding this comment.
- We use the state transition which does so implicitly , @briankariuki , I used some of your code
<%= state =
if(
is_atom(@story.photo.state),
do: @story.photo.state,
else: String.to_atom(@story.photo.state)
) %>
``` , do you mind chipping in here?
There was a problem hiding this comment.
Is there code somewhere which writes into the database while bypassing the validation?
My bad. I just re-read your comment. So in the DB everything is fine. hmmm... does seem like an Ash bug. Please ask in the elixirforum.com if there is an other explanation for this and add a link to that question here.
In addition:
- Add a comment in the code which describes why you are doing this. Otherwise somebody will remove that code in the future since it shouldn't be a problem in the first place.
- If possible write a test to make sure that we don't run into a problem if somebody deletes this code anyway.
- Resubmit the PR to be reviewed.
There was a problem hiding this comment.
I have added tests and comments for clarity.
I have also asked a question on elixir forum for the same https://elixirforum.com/t/using-database-values-stored-as-atoms-in-html-in-ash-framework/67437
|
@wintermeyer , kindly let me know once you deploy this |
I just triggered the deployment. ETA: +5 minutes |
|
All good , I can now see your face |
|
Seems like we have a couple of similar issues related to this bug causing photos not be seen , There will be a PR for this |
Can we add a test for this? |
|
Yes |
I noticed a bug where sometimes we have the state as "approved" , String , instead of :approved , atom , this PR ensures we only use the atom version of it.
