diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fa0bafde..2bba52116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog Fab-manager +## v5.5.5 2022 November 22 + +- Soft destroy of spaces and machines +- Fix a bug: in upgrade script, the error "the input device is not a TTY" is thrown when migrating the database +- Fix a bug: broken display of machines pages +- Fix a bug: some automated tests were randomly failing because ElasticSearch was not synced +- Fix a bug: payment related objects are not synced on Stripe when enabling the online payment module +- Fix a bug: unable set a main image of product and remove an image of product + ## v5.5.4 2022 November 17 - Fix a bug: unable to download an existing export of the statistics diff --git a/app/controllers/api/machines_controller.rb b/app/controllers/api/machines_controller.rb index a6baebc10..1879bd183 100644 --- a/app/controllers/api/machines_controller.rb +++ b/app/controllers/api/machines_controller.rb @@ -12,6 +12,8 @@ class API::MachinesController < API::ApiController def show @machine = Machine.includes(:machine_files, :projects).friendly.find(params[:id]) + + head :not_found if @machine.deleted_at end def create @@ -35,7 +37,11 @@ class API::MachinesController < API::ApiController def destroy authorize @machine - @machine.destroy + if @machine.destroyable? + @machine.destroy + else + @machine.soft_destroy! + end head :no_content end diff --git a/app/controllers/api/spaces_controller.rb b/app/controllers/api/spaces_controller.rb index 18a02cac5..75390c85d 100644 --- a/app/controllers/api/spaces_controller.rb +++ b/app/controllers/api/spaces_controller.rb @@ -6,11 +6,13 @@ class API::SpacesController < API::ApiController respond_to :json def index - @spaces = Space.includes(:space_image) + @spaces = Space.includes(:space_image).where(deleted_at: nil) end def show @space = Space.includes(:space_files, :projects).friendly.find(params[:id]) + + head :not_found if @space.deleted_at end def create @@ -36,7 +38,11 @@ class API::SpacesController < API::ApiController def destroy @space = get_space authorize @space - @space.destroy + if @space.destroyable? + @space.destroy + else + @space.soft_destroy! + end head :no_content end diff --git a/app/frontend/src/javascript/components/form/form-image-upload.tsx b/app/frontend/src/javascript/components/form/form-image-upload.tsx index 10c0db07b..2428992d7 100644 --- a/app/frontend/src/javascript/components/form/form-image-upload.tsx +++ b/app/frontend/src/javascript/components/form/form-image-upload.tsx @@ -60,11 +60,12 @@ export const FormImageUpload = , - { - attachment_name: f.name, - _destroy: false - } as UnpackNestedValue>> + `${id}.attachment_name` as Path, + f.name as UnpackNestedValue>> + ); + setValue( + `${id}._destroy` as Path, + false as UnpackNestedValue>> ); if (typeof onFileChange === 'function') { onFileChange({ attachment_name: f.name }); diff --git a/app/frontend/src/javascript/components/form/form-multi-file-upload.tsx b/app/frontend/src/javascript/components/form/form-multi-file-upload.tsx index 3746a9097..08a3fde47 100644 --- a/app/frontend/src/javascript/components/form/form-multi-file-upload.tsx +++ b/app/frontend/src/javascript/components/form/form-multi-file-upload.tsx @@ -6,9 +6,10 @@ import { FieldValues } from 'react-hook-form/dist/types/fields'; import { FormComponent, FormControlledComponent } from '../../models/form-component'; import { AbstractFormItemProps } from './abstract-form-item'; import { UseFormSetValue } from 'react-hook-form/dist/types/form'; -import { ArrayPath, FieldArray, useFieldArray } from 'react-hook-form'; +import { ArrayPath, FieldArray, Path, useFieldArray, useWatch } from 'react-hook-form'; import { FileType } from '../../models/file'; import { UnpackNestedValue } from 'react-hook-form/dist/types'; +import { FieldPathValue } from 'react-hook-form/dist/types/path'; interface FormMultiFileUploadProps extends FormComponent, FormControlledComponent, AbstractFormItemProps { setValue: UseFormSetValue, @@ -20,13 +21,26 @@ interface FormMultiFileUploadProps extend * This component allows to upload multiple files, in forms managed by react-hook-form. */ export const FormMultiFileUpload = ({ id, className, register, control, setValue, formState, addButtonLabel, accept }: FormMultiFileUploadProps) => { - const { fields, append, remove } = useFieldArray({ control, name: id as ArrayPath }); + const { append } = useFieldArray({ control, name: id as ArrayPath }); + const output = useWatch({ control, name: id as Path }); + + /** + * Remove an file + */ + const handleRemoveFile = (file: FileType, index: number) => { + return () => { + setValue( + `${id}.${index}._destroy` as Path, + true as UnpackNestedValue>> + ); + }; + }; return (
- {fields.map((field: FileType, index) => ( - ( + remove(index)}/> + onFileRemove={() => handleRemoveFile(field, index)}/> ))}
exten * This component allows to upload multiple images, in forms managed by react-hook-form. */ export const FormMultiImageUpload = ({ id, className, register, control, setValue, formState, addButtonLabel }: FormMultiImageUploadProps) => { - const { fields, append, remove } = useFieldArray({ control, name: id as ArrayPath }); + const { append } = useFieldArray({ control, name: id as ArrayPath }); const output = useWatch({ control, name: id as Path }); /** @@ -44,14 +44,10 @@ export const FormMultiImageUpload = >> ); } - if (typeof image.id === 'string') { - remove(index); - } else { - setValue( - `${id}.${index}._destroy` as Path, - true as UnpackNestedValue>> - ); - } + setValue( + `${id}.${index}._destroy` as Path, + true as UnpackNestedValue>> + ); }; }; @@ -74,8 +70,8 @@ export const FormMultiImageUpload =
- {fields.map((field: ImageType, index) => ( - ( + = ({ user, machine, onShowMachine, * Return the machine's picture or a placeholder */ const machinePicture = (): ReactNode => { - if (!machine.machine_image_attributes) { + if (!machine.machine_image) { return
; } return ( -
+
); }; diff --git a/app/frontend/src/javascript/models/machine.ts b/app/frontend/src/javascript/models/machine.ts index 43b1a143a..da506c0d7 100644 --- a/app/frontend/src/javascript/models/machine.ts +++ b/app/frontend/src/javascript/models/machine.ts @@ -1,6 +1,5 @@ import { Reservation } from './reservation'; import { ApiFilter } from './api'; -import { FileType } from './file'; export interface MachineIndexFilter extends ApiFilter { disabled: boolean, @@ -13,8 +12,12 @@ export interface Machine { spec?: string, disabled: boolean, slug: string, - machine_image_attributes: FileType, - machine_files_attributes?: Array, + machine_image: string, + machine_files_attributes?: Array<{ + id: number, + attachment: string, + attachment_url: string + }>, trainings?: Array<{ id: number, name: string, diff --git a/app/models/machine.rb b/app/models/machine.rb index 661f37256..0abd5e1c3 100644 --- a/app/models/machine.rb +++ b/app/models/machine.rb @@ -82,6 +82,10 @@ class Machine < ApplicationRecord reservations.empty? end + def soft_destroy! + update(deleted_at: DateTime.current) + end + def packs?(user) prepaid_packs.where(group_id: user.group_id) .where(disabled: [false, nil]) diff --git a/app/models/product.rb b/app/models/product.rb index d90c02f0e..e7921a56a 100644 --- a/app/models/product.rb +++ b/app/models/product.rb @@ -15,7 +15,7 @@ class Product < ApplicationRecord accepts_nested_attributes_for :product_files, allow_destroy: true, reject_if: :all_blank has_many :product_images, as: :viewable, dependent: :destroy - accepts_nested_attributes_for :product_images, allow_destroy: true, reject_if: ->(i) { i[:attachment].blank? } + accepts_nested_attributes_for :product_images, allow_destroy: true, reject_if: ->(i) { i[:attachment].blank? && i[:id].blank? } has_many :product_stock_movements, dependent: :destroy accepts_nested_attributes_for :product_stock_movements, allow_destroy: true, reject_if: :all_blank diff --git a/app/models/space.rb b/app/models/space.rb index 228c5fd5f..934b51754 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -66,6 +66,10 @@ class Space < ApplicationRecord reservations.empty? end + def soft_destroy! + update(deleted_at: DateTime.current) + end + private def update_gateway_product diff --git a/app/policies/machine_policy.rb b/app/policies/machine_policy.rb index 069da4f45..5a1ac8481 100644 --- a/app/policies/machine_policy.rb +++ b/app/policies/machine_policy.rb @@ -11,6 +11,6 @@ class MachinePolicy < ApplicationPolicy end def destroy? - user.admin? and record.destroyable? + user.admin? end end diff --git a/app/policies/space_policy.rb b/app/policies/space_policy.rb index d4174200d..6c69c0faa 100644 --- a/app/policies/space_policy.rb +++ b/app/policies/space_policy.rb @@ -1,3 +1,6 @@ +# frozen_string_literal: true + +# Check the access policies for API::SpacesController class SpacePolicy < ApplicationPolicy def create? user.admin? @@ -8,6 +11,6 @@ class SpacePolicy < ApplicationPolicy end def destroy? - user.admin? and record.destroyable? + user.admin? end end diff --git a/app/services/cart/set_offer_service.rb b/app/services/cart/set_offer_service.rb index d9bd11b1a..18a18c706 100644 --- a/app/services/cart/set_offer_service.rb +++ b/app/services/cart/set_offer_service.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +# module definition +module Cart; end + # Provides methods for set offer to item in cart class Cart::SetOfferService def call(order, orderable, is_offered) diff --git a/app/services/machine_service.rb b/app/services/machine_service.rb index 02f18c4f1..bc5720c8d 100644 --- a/app/services/machine_service.rb +++ b/app/services/machine_service.rb @@ -9,6 +9,9 @@ class MachineService else Machine.includes(:machine_image, :plans).order(sort_by) end + # do not include soft destroyed + machines = machines.where(deleted_at: nil) + if filters[:disabled].present? state = filters[:disabled] == 'false' ? [nil, false] : true machines = machines.where(disabled: state) diff --git a/app/services/setting_service.rb b/app/services/setting_service.rb index e8b1ba092..c90048ac6 100644 --- a/app/services/setting_service.rb +++ b/app/services/setting_service.rb @@ -46,7 +46,7 @@ class SettingService # sync all objects on stripe def sync_stripe_objects(setting) - return unless setting.name == 'stripe_secret_key' + return unless %w[stripe_secret_key online_payment_module].include?(setting.name) SyncObjectsOnStripeWorker.perform_async(setting.history_values.last&.invoicing_profile&.user&.id) end diff --git a/app/views/api/machines/index.json.jbuilder b/app/views/api/machines/index.json.jbuilder index e86436e0b..b9e8f5b76 100644 --- a/app/views/api/machines/index.json.jbuilder +++ b/app/views/api/machines/index.json.jbuilder @@ -1,5 +1,7 @@ # frozen_string_literal: true json.array!(@machines) do |machine| - json.partial! 'api/machines/machine', machine: machine + json.extract! machine, :id, :name, :slug, :disabled + + json.machine_image machine.machine_image.attachment.medium.url if machine.machine_image end diff --git a/app/views/api/machines/show.json.jbuilder b/app/views/api/machines/show.json.jbuilder index 099a136f3..2b271f395 100644 --- a/app/views/api/machines/show.json.jbuilder +++ b/app/views/api/machines/show.json.jbuilder @@ -1,11 +1,11 @@ # frozen_string_literal: true -json.partial! 'api/machines/machine', machine: @machine -json.extract! @machine, :description, :spec +json.extract! @machine, :id, :name, :description, :spec, :disabled, :slug +json.machine_image @machine.machine_image.attachment.large.url if @machine.machine_image json.machine_files_attributes @machine.machine_files do |f| json.id f.id - json.attachment_name f.attachment_identifier + json.attachment f.attachment_identifier json.attachment_url f.attachment_url end json.trainings @machine.trainings.each, :id, :name, :disabled diff --git a/app/views/api/products/_product.json.jbuilder b/app/views/api/products/_product.json.jbuilder index f5f143349..961baf234 100644 --- a/app/views/api/products/_product.json.jbuilder +++ b/app/views/api/products/_product.json.jbuilder @@ -4,12 +4,12 @@ json.extract! product, :id, :name, :slug, :sku, :is_active, :product_category_id :low_stock_threshold, :machine_ids, :created_at json.description sanitize(product.description) json.amount product.amount / 100.0 if product.amount.present? -json.product_files_attributes product.product_files do |f| +json.product_files_attributes product.product_files.order(created_at: :asc) do |f| json.id f.id json.attachment_name f.attachment_identifier json.attachment_url f.attachment_url end -json.product_images_attributes product.product_images do |f| +json.product_images_attributes product.product_images.order(created_at: :asc) do |f| json.id f.id json.attachment_name f.attachment_identifier json.attachment_url f.attachment_url diff --git a/app/workers/sync_objects_on_stripe_worker.rb b/app/workers/sync_objects_on_stripe_worker.rb index 62cf8826b..a1950a8b4 100644 --- a/app/workers/sync_objects_on_stripe_worker.rb +++ b/app/workers/sync_objects_on_stripe_worker.rb @@ -7,22 +7,45 @@ class SyncObjectsOnStripeWorker sidekiq_options lock: :until_executed, on_conflict: :reject, queue: :stripe def perform(notify_user_id = nil) + unless Stripe::Helper.enabled? + Rails.logger.warn('A request to sync payment related objects on Stripe will not run because Stripe is not enabled') + return false + end + + w = StripeWorker.new + sync_customers(w) + sync_coupons + sync_objects(w) + + Rails.logger.info 'Sync is done' + return unless notify_user_id + + Rails.logger.info "Notify user #{notify_user_id}" + user = User.find(notify_user_id) + NotificationCenter.call type: :notify_admin_objects_stripe_sync, + receiver: user, + attached_object: user + end + + def sync_customers(worker) Rails.logger.info 'We create all non-existing customers on stripe. This may take a while...' total = User.online_payers.count User.online_payers.each_with_index do |member, index| Rails.logger.info "#{index} / #{total}" begin stp_customer = member.payment_gateway_object&.gateway_object&.retrieve - StripeWorker.new.create_stripe_customer(member.id) if stp_customer.nil? || stp_customer[:deleted] + worker.perform(:create_stripe_customer, member.id) if stp_customer.nil? || stp_customer[:deleted] rescue Stripe::InvalidRequestError begin - StripeWorker.new.create_stripe_customer(member.id) + worker.perform(:create_stripe_customer, member.id) rescue Stripe::InvalidRequestError => e Rails.logger.error "Unable to create the customer #{member.id} do to a Stripe error: #{e}" end end end + end + def sync_coupons Rails.logger.info 'We create all non-existing coupons on stripe. This may take a while...' total = Coupon.all.count Coupon.all.each_with_index do |coupon, index| @@ -35,23 +58,16 @@ class SyncObjectsOnStripeWorker Rails.logger.error "Unable to create coupon #{coupon.code} on stripe: #{e}" end end + end - w = StripeWorker.new + def sync_objects(worker) [Machine, Training, Space, Plan].each do |klass| Rails.logger.info "We create all non-existing #{klass} on stripe. This may take a while..." total = klass.all.count klass.all.each_with_index do |item, index| Rails.logger.info "#{index} / #{total}" - w.perform(:create_or_update_stp_product, klass.name, item.id) + worker.perform(:create_or_update_stp_product, klass.name, item.id) end end - Rails.logger.info 'Sync is done' - return unless notify_user_id - - Rails.logger.info "Notify user #{notify_user_id}" - user = User.find(notify_user_id) - NotificationCenter.call type: :notify_admin_objects_stripe_sync, - receiver: user, - attached_object: user end end diff --git a/db/migrate/20221122123557_add_deleted_at_to_machine.rb b/db/migrate/20221122123557_add_deleted_at_to_machine.rb new file mode 100644 index 000000000..23107b653 --- /dev/null +++ b/db/migrate/20221122123557_add_deleted_at_to_machine.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# Allow soft destroy of machines. +# Machines with existing reservation cannot be destroyed because we need them for rebuilding invoices, statistics, etc. +# This attribute allows to make a "soft destroy" of a Machine, marking it as destroyed so it doesn't appear anymore in +# the interface (as if it was destroyed) but still lives in the database so we can use it to build data. +class AddDeletedAtToMachine < ActiveRecord::Migration[5.2] + def change + add_column :machines, :deleted_at, :datetime + add_index :machines, :deleted_at + end +end diff --git a/db/migrate/20221122123605_add_deleted_at_to_space.rb b/db/migrate/20221122123605_add_deleted_at_to_space.rb new file mode 100644 index 000000000..f5f13b0aa --- /dev/null +++ b/db/migrate/20221122123605_add_deleted_at_to_space.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# Allow soft destroy of spaces. +# Spaces with existing reservation cannot be destroyed because we need them for rebuilding invoices, statistics, etc. +# This attribute allows to make a "soft destroy" of a Space, marking it as destroyed so it doesn't appear anymore in +# the interface (as if it was destroyed) but still lives in the database so we can use it to build data. +class AddDeletedAtToSpace < ActiveRecord::Migration[5.2] + def change + add_column :spaces, :deleted_at, :datetime + add_index :spaces, :deleted_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 9e6a688de..effaba6fc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_10_03_133019) do +ActiveRecord::Schema.define(version: 2022_11_22_123605) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" @@ -358,6 +358,8 @@ ActiveRecord::Schema.define(version: 2022_10_03_133019) do t.datetime "updated_at" t.string "slug" t.boolean "disabled" + t.datetime "deleted_at" + t.index ["deleted_at"], name: "index_machines_on_deleted_at" t.index ["slug"], name: "index_machines_on_slug", unique: true end @@ -878,6 +880,8 @@ ActiveRecord::Schema.define(version: 2022_10_03_133019) do t.datetime "updated_at", null: false t.text "characteristics" t.boolean "disabled" + t.datetime "deleted_at" + t.index ["deleted_at"], name: "index_spaces_on_deleted_at" end create_table "spaces_availabilities", id: :serial, force: :cascade do |t| diff --git a/doc/sso_open_id_connect.md b/doc/sso_open_id_connect.md index e44c3893e..a0443ebc5 100644 --- a/doc/sso_open_id_connect.md +++ b/doc/sso_open_id_connect.md @@ -21,3 +21,12 @@ to the corresponding OpenID Connect claims: - profile.address To use the automatic mapping, add one of the fields above and click on the magic wand button near to the "Userinfo claim" input. + +## Known issues + +``` +Not found. Authentication passthru. +``` +This issue may occur if you have misconfigured the environment variable `DEFAULT_HOST` and/or `DEFAULT_PROTOCOL`. +Especially, if you have an automatic redirection (e.g. from example.org to example.com), `DEFAULT_HOST` *MUST* be configured with the redirection target (here example.com). +Once you have reconfigured these variables, please switch back the active authentication provider to FabManager, restart the application, then delete the OIDC provider you configured and re-create a new one for the new settings to be used. diff --git a/package.json b/package.json index 6ff9014a2..264204554 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fab-manager", - "version": "5.5.4", + "version": "5.5.5", "description": "Fab-manager is the FabLab management solution. It provides a comprehensive, web-based, open-source tool to simplify your administrative tasks and your marker's projects.", "keywords": [ "fablab", diff --git a/setup/upgrade.sh b/setup/upgrade.sh index c8c62c3dd..143b8ab46 100644 --- a/setup/upgrade.sh +++ b/setup/upgrade.sh @@ -296,7 +296,7 @@ upgrade() fi done compile_assets - if ! docker-compose run --rm "$SERVICE" bundle exec rake db:migrate; then + if ! docker-compose run --rm "$SERVICE" bundle exec rake db:migrate