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

Fix broken conference registering when no registration types #12706

Merged

Conversation

BarbaraOliveira13
Copy link
Contributor

@BarbaraOliveira13 BarbaraOliveira13 commented Apr 18, 2024

🎩 What? Why?

Fix conferences registers when the registration types are empty and registration is enabled

📌 Related Issues

Link your PR to an issue

Testing

  1. go to admin dashboard
  2. create a conference with registrations enabled and without registration_types
  3. publish the conference
  4. click on See conference button
  5. click on Register button
  6. can access to the registration page

📷 Screenshots

Please add screenshots of the changes you are proposing
Description

co-writter: @Stef-Rousset

♥️ Thank you!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']

@BarbaraOliveira13 BarbaraOliveira13 changed the title Fix/conference registering Fix broken conference registering when no registration types Apr 18, 2024
@BarbaraOliveira13 BarbaraOliveira13 marked this pull request as ready for review April 18, 2024 09:26
@Quentinchampenois Quentinchampenois marked this pull request as draft April 18, 2024 16:03
…1221)

* fix: hide register button in conference when no registration types

* lint: add missing space in conference show view page
@BarbaraOliveira13 BarbaraOliveira13 marked this pull request as ready for review April 19, 2024 14:20
@andreslucena andreslucena added type: bug Issues that describe a bug module: conferences type: fix PRs that implement a fix for a bug and removed type: bug Issues that describe a bug labels Apr 22, 2024
github-actions[bot]
github-actions bot previously approved these changes Apr 22, 2024
@andreslucena andreslucena self-assigned this Apr 22, 2024
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

@BarbaraOliveira13 @Stef-Rousset great fix and welcome to Decidim!

I could find an edge case although: what happens when the registration type is not published? As far as I could see, we're showing a buggy behavior: showing the Register button but then the RoutingError in the page. I think this could be fixed with the following snippet:

diff --git a/decidim-conferences/app/models/decidim/conference.rb b/decidim-conferences/app/models/decidim/conference.rb
index 1133c5c18d..936ceefa72 100644
--- a/decidim-conferences/app/models/decidim/conference.rb
+++ b/decidim-conferences/app/models/decidim/conference.rb
@@ -113,6 +113,12 @@ module Decidim
       available_slots > conference_registrations.count
     end

+    def has_published_registration_types?
+      return false if registration_types.empty?
+
+      registration_types.any?(&:published_at?)
+    end
+
     def remaining_slots
       available_slots - conference_registrations.count
     end
diff --git a/decidim-conferences/app/views/decidim/conferences/conferences/_conference_hero.html.erb b/decidim-conferences/app/views/decidim/conferences/conferences/_conference_hero.html.erb
index b2e617f920..a974d508e2 100644
--- a/decidim-conferences/app/views/decidim/conferences/conferences/_conference_hero.html.erb
+++ b/decidim-conferences/app/views/decidim/conferences/conferences/_conference_hero.html.erb
@@ -19,7 +19,7 @@
       <% if current_participatory_space.registrations_enabled? %>
         <% if current_participatory_space.has_registration_for?(current_user) %>
           <%= link_to t("layouts.decidim.conference_hero.manage_registration"), decidim_conferences.conference_registration_types_path(current_participatory_space), class: "button button__lg button__primary" %>
-        <% elsif current_participatory_space.registration_types.present? %>
+        <% elsif current_participatory_space.has_published_registration_types? %>
           <%= link_to t("layouts.decidim.conference_hero.register"), decidim_conferences.conference_registration_types_path(current_participatory_space), class: "button button__lg button__secondary" %>
         <% end %>
       <% end %>

You'd need to implement it in the other places where we're using the present? and make some specs too. Can you apply this or other solution to the bug that

@Stef-Rousset
Copy link
Contributor

Hello @andreslucena, thank you for your welcome and for the comment ! We are going to update the code and apply your suggestions, and add tests of course.

@BarbaraOliveira13 , @Stef-Rousset

github-actions[bot]
github-actions bot previously approved these changes Apr 25, 2024
@Stef-Rousset
Copy link
Contributor

Hello @andreslucena , we have applied your recommandations, thx again !

@BarbaraOliveira13 @Stef-Rousset

Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

A few tiny suggestions and this is ready to be merged. Great job guys!

Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@andreslucena andreslucena merged commit 3722ef4 into decidim:develop May 6, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: conferences type: fix PRs that implement a fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conference registering is broken when no registration_types
4 participants