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.

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

Petits nettoyages de RecurrenceConcern #5159

Open
wants to merge 4 commits into
base: production
Choose a base branch
from

Conversation

francois-ferrandis
Copy link
Contributor

Ces petits changements ont été extraits de #5065, où ils ne sont pas nécessaires. Il s'agit de changements sans impact fonctionnel, simplement du nettoyage.

@@ -68,12 +68,16 @@ def first_occurrence_ends_at
end
end

def recurrence=(hash)
super(hash.presence) # on évite d'avoir un objet Montrose::Recurrence avec une config vide
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ce setter nous éviter d'avoir un callback clear_empty_recurrence qui est seulement appelé before_save, ce qui signifie qu'on pouvait avoir

plage = PlageOuverture.new(..., recurrence: {})
plage.recurring?
# => true
plage.save
plage.recurring?
=> false

On évite donc ainsi d'être dans un état invalide, plutôt que de corriger cet état invalide juste avant de save.

def duration
(first_occurrence_ends_at - starts_at).to_i
end

def exceptionnelle?
recurrence.nil?
!recurring?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jusqu'ici, si on avait recurrence == {}, alors on avait en même temps exceptionnelle? => true et recurring? => true.

Comment on lines -91 to -96
def recurrence_interval
return nil if recurrence.nil?

recurrence.to_hash[:interval] || 1 # when interval is nil, it means 1
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sauf erreur de ma part, cette méthode n'est utilisée nulle part.

@@ -6,7 +6,7 @@ module RecurrenceConcern
attribute :start_time, :time_only # uses Tod::TimeOfDayType
attribute :end_time, :time_only # uses Tod::TimeOfDayType

before_save :clear_empty_recurrence, :set_recurrence_ends_at
before_save { self.recurrence_ends_at = recurrence&.ends_at&.end_of_day }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vu la longueur du code je préfère inliner plutôt que d'avoir de l'indirection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

1 participant