Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add extra validations for links, trackers; and news #385

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add extra validations for links, trackers; and news #385

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d6ba4d0
Add extra validations for links, trackers; and news
Oumph Mar 10, 2024
84db414
validates lang selection for bookmarks and links
Trim Apr 11, 2024
ec79de0
bookmarks: do not render preview and save button if bookmark is invalid
Trim Apr 11, 2024
7a706af
news: validate association to section and show preview only if valid
Trim Apr 11, 2024
2552255
only show preview and save button if content is valid
Trim Apr 11, 2024
676b0ef
add validates_associated for fields user have to choose
Trim Apr 11, 2024
e7dffa7
fix typo while copying test from model/link
Trim Apr 11, 2024
8a91804
remove unused lang_validation for links
Trim Apr 11, 2024
46aeada
tracker: category must be present (update model to follow form)
Trim Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/models/bookmark.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ class Bookmark < Content
belongs_to :owner, class_name: 'User'

validates :title, presence: { message: "Le titre est obligatoire" },
length: { maximum: 100, message: "Le titre est trop long" }
length: { maximum: 100, message: "Le titre est trop long" },
Oumph marked this conversation as resolved.
Show resolved Hide resolved
uniqueness: { message: "Un lien avec le même titre a déjà été proposé" }
validates :link, presence: { message: "Vous ne pouvez pas poster un lien vide" },
http_url: { message: "Le lien n'est pas valide" },
length: { maximum: 255, message: "Le lien est trop long" }
length: { maximum: 255, message: "Le lien est trop long" },
uniqueness: { message: "Le lien a déjà été proposé" }
Trim marked this conversation as resolved.
Show resolved Hide resolved

def link=(raw)
raw.strip!
Expand Down
8 changes: 7 additions & 1 deletion app/models/link.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ class Link < ActiveRecord::Base
Accessible = [:id, :user, :title, :url, :lang]

validates :title, presence: { message: "Un lien doit obligatoirement avoir un titre" },
length: { maximum: 100, message: "Le titre est trop long" }
length: { maximum: 100, message: "Le titre du lien est trop long" }
validates :url, http_url: { protocols: PROTOCOLS, message: "L'adresse n'est pas valide" },
presence: { message: "Un lien doit obligatoirement avoir une adresse" },
length: { maximum: 255, message: "L’adresse est trop longue" }
validate :lang_validation

def url=(raw)
raw.strip!
Expand All @@ -44,6 +45,11 @@ def url=(raw)
write_attribute :url, raw
end

def lang_validation
if lang == "xx"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il faudrait aussi tester si la valeur est une string vide (c'est possible en ouvrant la console du navigateur et en supprimant l'attribut required du <input>).

En fait, la validation devrait vérifier si la valeur appartient aux langues possibles et refuser si ce n'est pas le cas où si c'est la langue xx.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pour la langue, malheureusement, c'est plus compliqué à définir l'association que pour la section, parce qu'on ne peut pas utiliser validates_associated_to: en base de donnée, la langue est juste une string de 2 caractères.

Je n'ai pas compris pourquoi les langues ne sont définies que dans la base Redis et non pas dans la base mysql, c'est dommage :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, c'est bon, j'ai trouvé sur StackOverflow une solution théorique avec validates_inclusion_of: https://stackoverflow.com/a/4730085

L'idée serait de tester directement dans le modèle de Bookmark, l'inclusion dans la liste générée par Lang sans la valeur "xx".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, c'est bon, j'ai trouvé sur StackOverflow une solution théorique avec validates_inclusion_of: https://stackoverflow.com/a/4730085

C'est fait et j'ai ajouté le test dans models/link et dans models/bookmark pour les liens des dépêches et la section lien.

errors.add(:lang, "La langue du lien doit être définie") unless title.blank? or url.blank?
end
end
### Behaviour ###

def self.hit(id)
Expand Down
2 changes: 1 addition & 1 deletion app/views/bookmarks/_form.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
= form.text_field :link, autocomplete: 'off', required: 'required', spellcheck: 'false', maxlength: 1024
%p
= form.label :lang, "Langue"
= form.select :lang, Lang.all
= form.select :lang, Lang.all, { include_blank: true }, { required: "required" }
%p
- if form.object.new_record?
%p
Expand Down
2 changes: 1 addition & 1 deletion app/views/news/_form.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
= form.text_field :title, autocomplete: 'off', required: 'required', spellcheck: 'true', maxlength: 100
%p
= form.label :section_id, "Section de la dépêche"
= form.collection_select :section_id, Section.published, :id, :title
= form.collection_select :section_id, Section.published, :id, :title, { include_blank: true }, { required: "required" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a aussi besoin d'ajouter la même validation pour la section dans ce controlleur.

Si je procède à la même manipulation (enlever l'attribut required), alors, j'ai cette erreur qui apparaît:

image

Ça serait plus sympa d'avoir un bon message d'erreur du controlleur pour éviter de perdre le contenu de la dépêche soumise sans espace de rédaction :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Théoriquement, pour valider une association à une table (donc, pour la section de news ici), on peut utiliser les propriétés: validates_associated (validation de l'appartenance à la liste) et validates_presence_of (validation de la présence de la valeur) ensemble.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai ajouté "validates_associated" et j'ai changé le système de "prévisualisation" pour ne prévisualiser que si le contenu est valide.

La capture d'écran d'erreur que j'ai posté plus haut était due au fait que l'on essaie de faire la preview d'une news sans section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On peut envoyer en Anonyme en modération une dépêche avec un lien sans langue définie.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On peut injecter des langues inconnues sur un lien de dépêche en Anonyme (et ensuite le site essaie d'afficher une image inexistante).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En rédaction, on peut générer des 500 en envoyant des identifiants de section invalide pour la dépêche en cours.

%p
= form.label :wiki_body, "Contenu de la dépêche"
= form.text_area :wiki_body, required: 'required', spellcheck: 'true', class: 'markItUp'
Expand Down
2 changes: 1 addition & 1 deletion app/views/trackers/_form.html.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
= form.text_field :title, autocomplete: 'off', required: 'required', spellcheck: 'true', maxlength: 100
%p
= form.label :category_id, "Catégorie"
= form.collection_select :category_id, Category.all, :id, :title
= form.collection_select :category_id, Category.all, :id, :title, { include_blank: true }, { required: "required" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je n'ai pas eu le temps de testé, mais j'imagine qu'il y aura le même problème que pour la section de news.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Encore un 500 si on soumet avec un id de catégorie inconnu.

ActiveRecord::InvalidForeignKey (Mysql2::Error: Cannot add or update a child row: a foreign key constraint fails (linux fr.trackers, CONSTRAINT fk_trackers_on_category_id FOREIGN KEY (category_id) REFERENCES categori es (id)): INSERT INTO trackers (...)

- if @tracker.new_record?
%p.pot_de_miel
= form.label :pot_de_miel, "Ne pas remplir ce champ"
Expand Down