-
Notifications
You must be signed in to change notification settings - Fork 8
Display data providers #1631
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
Display data providers #1631
Changes from 4 commits
5cb296b
4f7abbd
384c355
33d4a3c
1876a2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| .data-provider-card-img { | ||
| background: white; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| padding: 1.5rem; | ||
| min-height: 9rem; | ||
| } | ||
|
|
||
| .data-provider-card-img img { | ||
| max-height: 6rem; | ||
| max-width: 80%; | ||
| object-fit: contain; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,14 @@ | ||
| class InstructorMenuComponent < ApplicationComponent | ||
| def initialize(show_drafts:, show_templates:, show_user_rights:) | ||
| def initialize(show_drafts:, show_templates:, show_user_rights:, show_definitions: false) | ||
| @show_drafts = show_drafts | ||
| @show_templates = show_templates | ||
| @show_user_rights = show_user_rights | ||
| @show_definitions = show_definitions | ||
| end | ||
|
|
||
| def render? | ||
| @show_drafts || @show_templates || @show_user_rights | ||
| @show_drafts || @show_templates || @show_user_rights || @show_definitions | ||
| end | ||
|
|
||
| attr_reader :show_drafts, :show_templates, :show_user_rights | ||
| attr_reader :show_drafts, :show_templates, :show_user_rights, :show_definitions | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <div class="fr-card fr-enlarge-link"> | ||
| <div class="fr-card__body"> | ||
| <div class="fr-card__content"> | ||
| <h2 class="fr-card__title"> | ||
| <%= link_to name, '#' %> | ||
| </h2> | ||
| <p class="fr-card__desc fr-text--xs"> | ||
| <code><%= slug %></code> | ||
| </p> | ||
| <div class="fr-card__end"> | ||
| <p class="fr-card__detail fr-icon-file-text-line"> | ||
| <%= t('instruction.data_providers.card.formulaires_count', count: definitions_count) %> | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <% if logo.attached? %> | ||
|
JeSuisUnCaillou marked this conversation as resolved.
|
||
| <div class="fr-card__header"> | ||
| <div class="fr-card__img data-provider-card-img"> | ||
| <%= image_tag logo, alt: name %> | ||
| </div> | ||
| </div> | ||
| <% end %> | ||
| </div> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| class Molecules::Instruction::DataProviders::CardComponent < ApplicationComponent | ||
| def initialize(data_provider:, definitions_count:) | ||
| @data_provider = data_provider | ||
| @definitions_count = definitions_count | ||
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :data_provider, :definitions_count | ||
|
|
||
| delegate :name, :slug, :logo, to: :data_provider | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| class Instruction::DataProvidersController < InstructionController | ||
| def index | ||
| authorize %i[instruction data_provider], :index? | ||
| @data_providers = policy_scope([:instruction, DataProvider]).with_attached_logo.order(:name) | ||
| @definitions_counts = @data_providers.index_with { |data_provider| reporter_definitions_count(data_provider) } | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def reporter_definitions_count(data_provider) | ||
| data_provider.authorization_definitions.count { |ad| current_user.reporter?(ad.id) } | ||
|
JeSuisUnCaillou marked this conversation as resolved.
|
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| class Instruction::DataProviderPolicy < ApplicationPolicy | ||
| class Scope < Scope | ||
| def resolve | ||
| return scope.all if user.admin? | ||
|
|
||
| scope.where(slug: user.roles_for(:reporter).provider_slugs) | ||
| end | ||
| end | ||
|
|
||
| def index? | ||
| user.reporter? || user.manager? | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <% set_title! t('page_titles.instruction_data_providers') %> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Le titre n'est toujours pas le bon, il devrait être "Fournisseur de Données - Instruction - DataPass". Ce n'est pas un affichage de "formulaires" ici. Ici, il nous manque un fil d'ariane et le titre reste à changer.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Je ferait le fil d'ariane dans une autre PR quand j'aurais toute la suite de pages à gérer, c'est prévu.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Je me suis déjà exprimé sur le nom de cette page, et la confusion autour du titre. Je vais pas faire une modif pour un truc que je vais jeter.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Si tu jettes la PR tu la close, c'est dans la liste à review. Et mon commentaire sera de toute façon pertinent pour l'autre page, si on affiche une liste de fournisseurs de données et que tu as quand même un titre qui dit formulaires, on serait quand même en NC
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non je veux pas la close, ma PR qui suit se base dessus
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Elle se merge pas dans develop, mais dans une branche qui résumera toute l'avancée sur la feature d'affichage des Formulaires et Cas d'usages. Donc selon moi on peut la merge et empiler la suite dessus, et il y en aura une dans la chaîne qui servira à retaper ce lien de "Formulaires" en une vraie liste de formulaires. |
||
|
|
||
| <h1 class="fr-my-5w"><%= t('.title') %></h1> | ||
|
|
||
| <% if @data_providers.empty? %> | ||
| <p class="fr-text--lg"><%= t('.empty') %></p> | ||
|
JeSuisUnCaillou marked this conversation as resolved.
|
||
| <% else %> | ||
| <div class="fr-grid-row fr-grid-row--gutters"> | ||
| <% @data_providers.each do |data_provider| %> | ||
| <div class="fr-col-12 fr-col-md-6 fr-col-lg-4"> | ||
| <%= render Molecules::Instruction::DataProviders::CardComponent.new(data_provider:, definitions_count: @definitions_counts[data_provider]) %> | ||
| </div> | ||
| <% end %> | ||
| </div> | ||
| <% end %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| # Context | ||
|
JeSuisUnCaillou marked this conversation as resolved.
|
||
|
|
||
| Read this PR : https://github.com/etalab/data_pass/pull/1595/ | ||
| This was a prototype of rough display of the formulaires (definitions) and cas d'usages (forms). | ||
|
|
||
| Then read this PR : https://github.com/etalab/data_pass/pull/1607. | ||
| This was built over the previous PR, and is a prototype of pretty display of the formulaires and cas d'usages. | ||
|
|
||
| Now my plan is to start over from develop to make these features, and I'll use the inspiration of this design : /home/caillou/Downloads/DataPass%20-%20Fiche%20&%20cas%20d'usage%20(standalone).html | ||
|
|
||
| But first, we are going to explore the code of the prototypes. | ||
|
|
||
| # Explore the prototypes | ||
|
|
||
| Explore the 2 PRs and tell me what should be reconsidered in the controllers and models to make it clean and respect the datapass' guidelines. | ||
|
|
||
| > Response was a very detailed review, but actually I first need a summary of what was done to the models | ||
|
|
||
| ## Actually | ||
|
|
||
| Actually, I first need a summary of the PR's changes to the models | ||
|
|
||
| ## Summary of the model changes | ||
|
|
||
| > Claude's answer | ||
|
|
||
| All model changes are in PR 1595 only — PR 1607 touched no models. Here's the summary: | ||
|
|
||
| --- | ||
|
|
||
| ### New model: `FormTemplate` | ||
|
|
||
| A new ActiveRecord model backed by the `form_templates` table. It sits between `HabilitationType` (DB) and `AuthorizationRequestForm` (façade). | ||
|
|
||
| **Schema:** `slug`, `name`, `description`, `introduction`, `use_case`, `default`, `public`, `startable_by_applicant`, `single_page_view`, `service_provider_id` (string, FK to YAML-backed `ServiceProvider`), plus jsonb columns `steps`, `static_blocks`, `scopes_config`, `initialize_with`. | ||
|
|
||
| **Key behaviors:** | ||
| - `friendly_id` on `name`, `paper_trail` versioning | ||
| - `belongs_to :habilitation_type` | ||
| - Validates that the slug doesn't collide with any YAML-defined form uid (`slug_not_taken_by_yaml`) | ||
| - Enforces exactly one `default: true` per `HabilitationType` (via `only_one_default_per_habilitation_type`, `ht_keeps_at_least_one_default`, and `ensure_not_last_default` on `before_destroy`) | ||
| - `after_commit :reset_arf_cache` — invalidates the `AuthorizationRequestForm` in-memory cache (via Redis) whenever a template is created, updated, or destroyed | ||
| - `service_provider` is resolved at read time via `ServiceProvider.find(service_provider_id)`, returning `nil` gracefully if not found | ||
|
|
||
| --- | ||
|
|
||
| ### Modified: `HabilitationType` | ||
|
|
||
| - Added `has_many :form_templates, dependent: :destroy` | ||
| - Added `after_create :ensure_default_form_template!` — when a new `HabilitationType` is created, it immediately creates one default `FormTemplate` with slug `"#{slug}-default"`, seeding it with empty jsonb columns (steps/scopes cascade from the HT at render time) | ||
| - `after_save :reset_static_caches` and `after_destroy :reset_static_caches` replaced by a single `after_commit :reset_static_caches, on: %i[create update destroy]` — avoids firing before the transaction is committed | ||
|
|
||
| --- | ||
|
|
||
| ### Modified: `AuthorizationRequestForm` (the YAML/DB façade) | ||
|
|
||
| The façade previously iterated `HabilitationType` directly; it now iterates `FormTemplate`. | ||
|
|
||
| - `db_records` now does `FormTemplate.includes(:habilitation_type).filter_map { build_form_from_template(template) }` instead of building one form per `HabilitationType`. This means one `HabilitationType` can now produce **N forms** (one per `FormTemplate`) rather than always exactly one. | ||
| - `build_form_from_template` maps all `FormTemplate` columns to form attributes, with cascading fallback to `HabilitationType` for `name`, `description`, `introduction`, and `steps` when the template columns are blank. | ||
| - New attribute `form_template` exposed on the façade to carry the originating `FormTemplate` record. | ||
| - `inherited?(field)` — returns `true` if the given field (`name`, `description`, `introduction`, `steps`) is blank on the underlying `FormTemplate` (i.e. it fell back to the `HabilitationType` value). | ||
| - `from_database?` — returns `true` if the form came from a `FormTemplate` (vs a YAML record). | ||
|
|
||
|
|
||
| --- | ||
|
|
||
| ## We dont' want the in-db cas d'usages (form_templates) yet. | ||
|
|
||
| Ok, so I understand the PR 1595 introduces a in-db model for the cas d'usages (form_templates), but I don't want to do that yet. The work I want to do is to _only_ display formulaires (definitions) and cas d'usages (forms) for the instructors and managers. | ||
|
|
||
| Can you summarize the model changes left if we ignore this ? (I think there should be none). Then summarize the controllers changes if we ignore this. | ||
|
|
||
| ## Model changes without `FormTemplate` | ||
|
|
||
| > Claude's answer | ||
|
|
||
| None. Every model change in PR 1595 exists purely to introduce and support `FormTemplate`: | ||
| - `HabilitationType` changes (`has_many :form_templates`, `after_create :ensure_default_form_template!`, callback timing fix) are all in service of `FormTemplate` | ||
| - `AuthorizationRequestForm` changes (`build_form_from_template`, `inherited?`, `from_database?`, `form_template` attribute) are all wiring `FormTemplate` into the façade | ||
|
|
||
| The existing `AuthorizationDefinition` + `AuthorizationRequestForm` (YAML-backed) are already sufficient to display formulaires and cas d'usages. Zero model changes needed. | ||
|
|
||
| --- | ||
|
|
||
| ## Controller changes, ignoring `FormTemplate` | ||
|
|
||
| > Claude's answer | ||
|
|
||
| Everything still applies — the controllers only read `AuthorizationDefinition` and `AuthorizationRequestForm`, neither of which requires `FormTemplate`. | ||
|
|
||
| **From PR 1595 (new infrastructure):** | ||
| - `Instruction::DataProvidersController` — `index` listing providers the current user has reporter access to | ||
| - `Instruction::AbstractCatalogueController` — `before_action :set_data_provider` (find by slug + authorize), shared parent for the two below | ||
| - `Instruction::FormulairesController` — `index` filtering `authorization_definitions` by policy, `show` loading `available_forms` and counts | ||
| - `Instruction::CasUsagesController` — `show` finding a specific form by uid within a formulaire | ||
| - 3 Pundit policies (`DataProviderPolicy`, `AuthorizationDefinitionPolicy`, `AuthorizationRequestFormPolicy`) | ||
| - Routes: `/instruction/fournisseurs-donnees` + nested formulaires + cas-usage | ||
|
|
||
| **From PR 1607 (visual prototype built on top):** | ||
| - `FormulairesController#show` extended with: `@authorization_request` stub (for WYSIWYG preview), stats counts, email count | ||
| - `CasUsagesController#index` added, `show` extended with stats counts | ||
| - `Instruction::EmailsAutomatiquesController` + `AutomaticEmailsCatalog` service — entirely independent of `FormTemplate` | ||
| - `DashboardController` — navigation shortcut to the provider/formulaire list | ||
| - Various views, partials, CSS, locales | ||
|
|
||
|
|
||
| # Let's go | ||
|
|
||
| We are ready to implement the display of definitions (formulaires) and forms (cas d'usages). | ||
|
|
||
| Understand that in French and in the UI, we are naming authorization_definitions "Formulaires" and forms "Cas d'usages". And we don't rename them in the codebase yet. | ||
|
|
||
| In the following prompts, I'll call "the prototype" the code in the 2 Pull Request #1595 and #1607 (mostly 1607). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
|
|
||
| > Read `/docs/shaping/display_formulaires/Explore prototype.md` for context. | ||
|
|
||
| # List data providers and their authorization_definitions | ||
|
|
||
| First, we are going to make the routes, controllers and views to list the data providers. | ||
| We will also add the button "Gérer mes formulaires" in /instruction/tableau-de-bord/demandes | ||
|
|
||
| Use the prototype as an example for the UI and the wordings, but you can rethink all the code. | ||
|
|
||
| Make ViewComponents like describe in @CLAUDE.md:133-141 | ||
|
|
||
| ## Submenu entry instead of button in dashboard | ||
|
|
||
| Instead of the button in the dashboard, I want a new submenu entry called "Formulaires", like in this design : file:///home/caillou/Apps/datagouv/data_pass/docs/shaping/display%20formulaires/designs/DataPass%20-%20Fiche%20&%20cas%20d'usage%20(standalone)%20(V0)%20(1).html | ||
|
|
||
| (the deisgn call it "Formulaires et cas d'usages", but we will just name it "Formulaires"). | ||
|
|
||
| ## A page to list the providers only | ||
|
|
||
| Ok, forget about the list of definitions, we will just list the providers. I want the design to look like this : https://sandbox.datapass.api.gouv.fr/instruction/fournisseurs-donnees (which is the PR #1607 you should have take as a model actually) | ||
|
|
||
| ## Tweaks | ||
|
|
||
| - We are missing the cursor:pointer on hover, and the blue arrow bottom right. Check out #1607 for that. | ||
|
|
||
| - We are not renaming authorization_definitions into formulaires yet ! Don't use "formulaire" where we are actually referencing authorization_definitions in the code. | ||
|
|
||
| - Don't use inlince css, make a class and put the css in a stylesheet. | ||
|
|
||
| - rename accessible_count into definitions_count in the card component | ||
|
|
||
| - I moved the feature file under /features/instructeurs/gestion_des_formulaires/ | ||
|
|
||
| - the view component preview without logo has an error : Couldn't find DataProvider with [WHERE "logo_attachment"."id" IS NULL]. It should instead render a card without logo. | ||
|
|
||
| - why does the view component preview doesn't look like what we see on /instruction/fournisseurs-donnees ? The logo is not centered, and overflows from its container. | ||
|
|
||
| - Add a feature to check if an instructor with only 2 data_providers in his roles sees only these two providers in the list, and another to check if an instructor with no data_providers in his roles but one definition, sees the provider of this definition. |
Uh oh!
There was an error while loading. Please reload this page.