diff --git a/app/models/queries/notifications/notification_query.rb b/app/models/queries/notifications/notification_query.rb index be6587a909..259a04669e 100644 --- a/app/models/queries/notifications/notification_query.rb +++ b/app/models/queries/notifications/notification_query.rb @@ -34,6 +34,6 @@ class Queries::Notifications::NotificationQuery < Queries::BaseQuery end def default_scope - Notification.recipient(user) + Notification.visible(User.current).recipient(user) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index e5d31dab64..bb15b05899 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -285,7 +285,7 @@ class Repository < ApplicationRecord elsif committer.strip =~ /\A([^<]+)(<(.*)>)?\z/ username = $1.strip email = $3 - u = User.find_by_login(username) + u = User.by_login(username).first u ||= User.find_by_mail(email) unless email.blank? user = u end diff --git a/docs/getting-started/projects/README.md b/docs/getting-started/projects/README.md index 1871cb4bca..bbc718a39e 100644 --- a/docs/getting-started/projects/README.md +++ b/docs/getting-started/projects/README.md @@ -4,7 +4,7 @@ sidebar_navigation: priority: 900 description: Introduction to projects in OpenProject. robots: index, follow -keywords: open project, create project, projects introduction +keywords: open project, create project, project introduction --- # Projects introduction diff --git a/docs/installation-and-operations/configuration/incoming-emails/README.md b/docs/installation-and-operations/configuration/incoming-emails/README.md index 8b1ef042c7..bdaa832f64 100644 --- a/docs/installation-and-operations/configuration/incoming-emails/README.md +++ b/docs/installation-and-operations/configuration/incoming-emails/README.md @@ -123,7 +123,7 @@ If a matching account is found, the mail handler impersonates the user to create If no matching account is found, the mail is rejected. To override this behavior and allow unknown mail address to create work packages, set the option `no_permission_check=1` and specify with `unknown_user=accept` -**Note**: This feature only provides a mapping of mail to user account, it does not authenticate the user based on the mail. Since you can easily spoof mail addresses, you should not rely on the authenticity of work packages created that way. +**Note**: This feature only provides a mapping of mail to user account, it does not authenticate the user based on the mail. Since you can easily spoof mail addresses, you should not rely on the authenticity of work packages created that way. At the moment in the OpenProject Enterprise Cloud work package generation by emails can only be triggered by registered email addresses. **Users with mail suffixes** diff --git a/docs/installation-and-operations/installation/manual/README.md b/docs/installation-and-operations/installation/manual/README.md index ffe30ea6cb..5be75dfa9b 100644 --- a/docs/installation-and-operations/installation/manual/README.md +++ b/docs/installation-and-operations/installation/manual/README.md @@ -381,7 +381,7 @@ Your OpenProject installation is ready to run. Please refer to the [Operation gu This step is optional. OpenProject can be extended by various plug-ins, which extend OpenProject's capabilities. -For general information and a list of all plug-ins known to us, refer to to the [plug-in page](https://community.openproject.org/projects/openproject/wiki/OpenProject_Plug-Ins). +For general information and a list of all plug-ins known to us, refer to to the [plug-in page](../../../system-admin-guide/integrations/). OpenProject plug-ins are separated in ruby gems. You can install them by listing them in a file called `Gemfile.plugins`. An example `Gemfile.plugins` file looks like this: diff --git a/frontend/src/app/features/projects/components/new-project/new-project.component.ts b/frontend/src/app/features/projects/components/new-project/new-project.component.ts index ae9640f1b3..1d33de4b59 100644 --- a/frontend/src/app/features/projects/components/new-project/new-project.component.ts +++ b/frontend/src/app/features/projects/components/new-project/new-project.component.ts @@ -43,7 +43,6 @@ export class NewProjectComponent extends UntilDestroyedMixin implements OnInit { hiddenFields:string[] = [ 'identifier', - 'sendNotifications', 'active', ]; @@ -114,6 +113,11 @@ export class NewProjectComponent extends UntilDestroyedMixin implements OnInit { } private isHiddenField(key:string|undefined):boolean { + // We explictly want to show the sendNotifications param + if (key === '_meta.sendNotifications') { + return false; + } + return !!key && (this.hiddenFields.includes(key) || this.isMeta(key)); } diff --git a/frontend/src/app/shared/components/forms/fieldset.sass b/frontend/src/app/shared/components/forms/fieldset.sass index 885bfed042..8e4b49395d 100644 --- a/frontend/src/app/shared/components/forms/fieldset.sass +++ b/frontend/src/app/shared/components/forms/fieldset.sass @@ -7,13 +7,12 @@ &_collapsible & &--toggle::before - display: inline-block - content: "" + @include icon-mixin-arrow-up1 padding: .625rem .25rem 0 &_collapsed & &--toggle::before - content: "" + @include icon-mixin-arrow-down1 &--fields height: 0 diff --git a/frontend/src/global_styles/content/_table.sass b/frontend/src/global_styles/content/_table.sass index 36aa5d6f58..3f804fd651 100644 --- a/frontend/src/global_styles/content/_table.sass +++ b/frontend/src/global_styles/content/_table.sass @@ -166,6 +166,9 @@ table.generic-table padding-bottom: 0.5rem vertical-align: top + &.form--td + vertical-align: middle + // Center input fields and select boxes vertically in tables .form--field margin: 0px diff --git a/modules/budgets/app/controllers/budgets_controller.rb b/modules/budgets/app/controllers/budgets_controller.rb index 2d47f2c1c8..f09bb8c9e6 100644 --- a/modules/budgets/app/controllers/budgets_controller.rb +++ b/modules/budgets/app/controllers/budgets_controller.rb @@ -239,8 +239,8 @@ class BudgetsController < ApplicationController } if current_user.allowed_to?(permission, project) - response["#{element_id}_costs_text"] = number_to_currency(costs) - response["#{element_id}_cost_value"] = unitless_currency_number(costs) + response["#{element_id}_costs"] = number_to_currency(costs) + response["#{element_id}_cost_value"] = response["#{element_id}_amount"] = unitless_currency_number(costs) end response diff --git a/modules/budgets/app/views/budgets/items/_budget_override_cost_form.html.erb b/modules/budgets/app/views/budgets/items/_budget_override_cost_form.html.erb new file mode 100644 index 0000000000..6737592ff2 --- /dev/null +++ b/modules/budgets/app/views/budgets/items/_budget_override_cost_form.html.erb @@ -0,0 +1,19 @@ + \ No newline at end of file diff --git a/modules/budgets/app/views/budgets/items/_labor_budget_item.html.erb b/modules/budgets/app/views/budgets/items/_labor_budget_item.html.erb index d487db2127..e24adf012c 100644 --- a/modules/budgets/app/views/budgets/items/_labor_budget_item.html.erb +++ b/modules/budgets/app/views/budgets/items/_labor_budget_item.html.erb @@ -61,19 +61,19 @@ See COPYRIGHT and LICENSE files for more details. inputmode: :decimal, placeholder: t(:label_example_placeholder, decimal: unitless_currency_number(1000.50)), class: 'budget-item-value form--text-field', - data: { :'request-key' => 'hours' } %> + data: {:'request-key' => 'hours'} %> <%= cost_form.select :user_id, - Principal.possible_assignee(@project).sort.map { |u| [u.name, u.id] }, - { prompt: true }, + Principal.possible_assignee(@project).sort.map {|u| [u.name, u.id]}, + {prompt: true}, { index: id_or_index, class: 'form--select budget-item-value', - data: { :'request-key' => 'user_id' } + data: {:'request-key' => 'user_id'} } %> @@ -82,29 +82,27 @@ See COPYRIGHT and LICENSE files for more details. <%= cost_form.text_field :comments, index: id_or_index, size: 40 %> <% if User.current.allowed_to?(:view_cost_rates, @project) %> - + <%# Keep current budget as hidden field because otherwise they will be overridden %> <% if templated == false && labor_budget_item.overridden_costs? %> <%= cost_form.hidden_field :amount, index: id_or_index, value: unitless_currency_number(labor_budget_item.amount) %> <% end %> <% cost_value = labor_budget_item.amount || labor_budget_item.calculated_costs(@budget.fixed_date, @budget.project_id) %> - <%= cost_form.hidden_field :currency, index: id_or_index, value: Setting.plugin_costs['costs_currency'] %> - " - data-placeholder="<%= t(:label_example_placeholder, decimal: unitless_currency_number(1000.50)) %>" %> - " class="costs--edit-planned-costs-btn icon-context icon-edit" title="<%= t(:help_click_to_edit) %>"> - <% if labor_budget_item.costs_visible_by?(User.current) %> - <%= cost_form.hidden_field :cost_value, index: id_or_index, value: unitless_currency_number(cost_value) %> - "> - <%= number_to_currency(cost_value) %> - - <% end %> - - + " class="costs--edit-planned-costs-btn icon-context icon-edit" title="<%= t(:help_click_to_edit) %>"> + <% if labor_budget_item.costs_visible_by?(User.current) %> + <%= number_to_currency(cost_value) %> + <% end %> + + <%= render partial: '/budgets/items/budget_override_cost_form', + locals: { + field_name: "#{name_prefix}[amount]", + cost_value: cost_value, + } + %> <% end %> - + <%= op_icon('icon-context icon-delete') %> <%= t(:button_delete) %> diff --git a/modules/budgets/app/views/budgets/items/_material_budget_item.html.erb b/modules/budgets/app/views/budgets/items/_material_budget_item.html.erb index 2eebec4478..2e06b95a21 100644 --- a/modules/budgets/app/views/budgets/items/_material_budget_item.html.erb +++ b/modules/budgets/app/views/budgets/items/_material_budget_item.html.erb @@ -61,11 +61,11 @@ See COPYRIGHT and LICENSE files for more details. inputmode: :decimal, placeholder: t(:label_example_placeholder, decimal: unitless_currency_number(1000.50)), class: 'budget-item-value form--text-field', - data: { :'request-key' => 'units' } %> + data: {:'request-key' => 'units'} %> - "> + "> <%= h material_budget_item.cost_type.unit_plural if material_budget_item.cost_type %> @@ -76,7 +76,7 @@ See COPYRIGHT and LICENSE files for more details. { index: id_or_index, class: 'form--select budget-item-value', - data: { :'request-key' => 'cost_type_id' } + data: {:'request-key' => 'cost_type_id'} } %> @@ -84,28 +84,26 @@ See COPYRIGHT and LICENSE files for more details. <%= cost_form.text_field :comments, index: id_or_index, size: 40 %> <% if User.current.allowed_to? :view_cost_rates, @project %> - + <%# Keep current budget as hidden field because otherwise they will be overridden %> <% if templated == false && material_budget_item.overridden_costs? %> <%= cost_form.hidden_field :amount, index: id_or_index, value: unitless_currency_number(material_budget_item.amount) %> <% end %> <% cost_value = material_budget_item.amount || material_budget_item.calculated_costs(@budget.fixed_date) %> - <%= cost_form.hidden_field :currency, index: id_or_index, value: Setting.plugin_costs['costs_currency'] %> - <%= cost_form.hidden_field :cost_value, index: id_or_index, value: unitless_currency_number(cost_value) %> - - " - data-placeholder="<%= t(:label_example_placeholder, decimal: unitless_currency_number(1000.50)) %>" %> - - "> - <%= number_to_currency(cost_value) %> - - - + + <%= number_to_currency(cost_value) %> + + + <%= render partial: '/budgets/items/budget_override_cost_form', + locals: { + field_name: "#{name_prefix}[amount]", + cost_value: cost_value, + } + %> <% end %> - + <%= op_icon('icon-context icon-delete') %> <%= t(:button_delete) %> diff --git a/modules/budgets/frontend/module/augment/cost-budget-subform.augment.service.ts b/modules/budgets/frontend/module/augment/cost-budget-subform.augment.service.ts index 1380b10398..4ecf606e27 100644 --- a/modules/budgets/frontend/module/augment/cost-budget-subform.augment.service.ts +++ b/modules/budgets/frontend/module/augment/cost-budget-subform.augment.service.ts @@ -1,4 +1,4 @@ -//-- copyright +// -- copyright // OpenProject is an open source project management software. // Copyright (C) 2012-2021 the OpenProject GmbH // @@ -26,18 +26,17 @@ // See COPYRIGHT and LICENSE files for more details. //++ -import { Injectable } from "@angular/core"; +import { Injectable } from '@angular/core'; import { HttpClient } from '@angular/common/http'; -import { HalResourceNotificationService } from "core-app/features/hal/services/hal-resource-notification.service"; +import { HalResourceNotificationService } from 'core-app/features/hal/services/hal-resource-notification.service'; @Injectable() export class CostBudgetSubformAugmentService { - constructor(private halNotification:HalResourceNotificationService, - private http:HttpClient) { + private http:HttpClient) { } - listen() { + listen():void { jQuery('costs-budget-subform').each((i, match) => { const el = jQuery(match); @@ -45,7 +44,7 @@ export class CostBudgetSubformAugmentService { const templateEl = el.find('.budget-row-template'); templateEl.detach(); const template = templateEl[0].outerHTML; - let rowIndex = parseInt(el.attr('item-count') as string); + let rowIndex = parseInt(el.attr('item-count') as string, 10); // Refresh row on changes el.on('change', '.budget-item-value', (evt) => { @@ -60,7 +59,7 @@ export class CostBudgetSubformAugmentService { }); // Add new row handler - el.find('.budget-add-row').click((evt) => { + el.find('.budget-add-row').on('click', (evt) => { evt.preventDefault(); const row = jQuery(template.replace(/INDEX/g, rowIndex.toString())); row.show(); @@ -75,30 +74,32 @@ export class CostBudgetSubformAugmentService { /** * Refreshes the given row after updating values */ - public refreshRow(el:JQuery, row_identifier:string) { - const row = el.find('#' + row_identifier); + public refreshRow(el:JQuery, row_identifier:string):void { + const row = el.find(`#${row_identifier}`); const request = this.buildRefreshRequest(row, row_identifier); this.http .post( - el.attr('update-url')!, + el.attr('update-url') as string, request, { - headers: { 'Accept': 'application/json' }, - withCredentials: true - }) + headers: { Accept: 'application/json' }, + withCredentials: true, + }, + ) .subscribe( (data:any) => { _.each(data, (val:string, selector:string) => { const element = document.getElementById(selector) as HTMLElement|HTMLInputElement|undefined; if (element instanceof HTMLInputElement) { + element.disabled = false; element.value = val; } else if (element) { element.textContent = val; } }); }, - (error:any) => this.halNotification.handleRawError(error) + (error:any) => this.halNotification.handleRawError(error), ); } @@ -106,15 +107,15 @@ export class CostBudgetSubformAugmentService { * Returns the params for the update request */ private buildRefreshRequest(row:JQuery, row_identifier:string) { - const request:any = { + const request:Record = { element_id: row_identifier, - fixed_date: jQuery('#budget_fixed_date').val() + fixed_date: jQuery('#budget_fixed_date').val(), }; // Augment common values with specific values for this type - row.find('.budget-item-value').each((_i:number, el:any) => { + row.find('.budget-item-value').each((i:number, el:Element) => { const field = jQuery(el); - request[field.data('requestKey')] = field.val() || '0'; + request[field.data('requestKey') as string] = field.val() || '0'; }); return request; diff --git a/modules/budgets/frontend/module/augment/planned-costs-form.ts b/modules/budgets/frontend/module/augment/planned-costs-form.ts index ccfb0cffef..f64c715522 100644 --- a/modules/budgets/frontend/module/augment/planned-costs-form.ts +++ b/modules/budgets/frontend/module/augment/planned-costs-form.ts @@ -27,85 +27,27 @@ //++ export class PlannedCostsFormAugment { - - public obj:JQuery; - public objId:string; - public objName:string; - public placeholder:string; - - static listen() { + static listen():void { jQuery(document).on('click', '.costs--edit-planned-costs-btn', (evt) => { - const form = jQuery(evt.target as any).closest('cost-unit-subform') as JQuery; - new PlannedCostsFormAugment(form); - }); - } - - constructor(public $element:JQuery) { - this.objId = this.$element.attr('obj-id')!; - this.objName = this.$element.attr('obj-name')!; - this.obj = jQuery(`#${this.objId}_costs`) as any; - this.placeholder = this.$element.data('placeholder'); - - this.makeEditable(); - } - - public makeEditable() { - this.edit_and_focus(); - } + const link = evt.target as HTMLElement; + const form = link.nextElementSibling as HTMLElement; - private edit_and_focus() { - this.edit(); + link.hidden = true; + form.hidden = false; - jQuery('#' + this.objId + '_costs_edit').trigger('focus'); - jQuery('#' + this.objId + '_costs_edit').trigger('select'); - } - - private getCurrency() { - return jQuery('#' + this.objId + '_currency').val(); - } - - private getValue() { - let costValueElement = jQuery('#' + this.objId + '_cost_value'); - return costValueElement.length > 0 ? costValueElement.val() : '0.00'; - } - - private edit() { - this.obj.hide(); - - const id = this.obj[0].id; - const currency = this.getCurrency(); - const value = this.getValue(); - const name = this.objName; - const placeholder = this.placeholder; + const input = form.querySelector('input') as HTMLInputElement; + input.disabled = false; + }); - const template = ` -
-
-
-
-
- -
-
${currency}
-
-
-
- `; + jQuery(document).on('click', '.costs--edit-planned-costs-cancel-btn', (evt) => { + const form = (evt.target as HTMLElement).closest('.costs--edit-form') as HTMLElement; + const link = form.previousElementSibling as HTMLElement; - jQuery(template).insertAfter(this.obj); + link.hidden = false; + form.hidden = true; - const that = this; - jQuery('#' + id + '_cancel').on('click', function () { - jQuery('#' + id + '_section').remove(); - that.obj.show(); - return false; + const input = form.querySelector('input') as HTMLInputElement; + input.disabled = true; }); } } - diff --git a/modules/budgets/spec/features/budgets/update_budget_spec.rb b/modules/budgets/spec/features/budgets/update_budget_spec.rb index 7404d04162..a13f63b799 100644 --- a/modules/budgets/spec/features/budgets/update_budget_spec.rb +++ b/modules/budgets/spec/features/budgets/update_budget_spec.rb @@ -162,7 +162,7 @@ describe 'updating a budget', type: :feature, js: true do # Open first item budget_page.open_edit_planned_costs! material_budget_item.id, type: :material - expect(page).to have_field("budget_existing_material_budget_item_attributes_#{material_budget_item.id}_costs_edit") + expect(page).to have_field("budget_existing_material_budget_item_attributes_#{material_budget_item.id}_amount") click_on 'OK' expect(budget_page).to have_content(I18n.t(:notice_successful_update, locale: :de)) diff --git a/modules/budgets/spec/support/pages/budget_form.rb b/modules/budgets/spec/support/pages/budget_form.rb index 881aab6e62..a694bc1a11 100644 --- a/modules/budgets/spec/support/pages/budget_form.rb +++ b/modules/budgets/spec/support/pages/budget_form.rb @@ -73,7 +73,7 @@ module Pages open_edit_planned_costs!(id, type: type) row_id = "#budget_existing_#{type}_budget_item_attributes_#{id}" - editor_name = "budget_existing_#{type}_budget_item_attributes_#{id}_costs_edit" + editor_name = "budget_existing_#{type}_budget_item_attributes_#{id}_amount" page.within row_id do fill_in editor_name, with: costs diff --git a/modules/costs/app/views/costlog/edit.html.erb b/modules/costs/app/views/costlog/edit.html.erb index fc9bcb6dce..cdab4102be 100644 --- a/modules/costs/app/views/costlog/edit.html.erb +++ b/modules/costs/app/views/costlog/edit.html.erb @@ -113,25 +113,27 @@ See COPYRIGHT and LICENSE files for more details. <% end %> - -
- - - <%= hidden_field_tag :currency, Setting.plugin_costs['costs_currency'], id: 'cost_entry_currency' %> - <% if User.current.allowed_to?(:view_cost_rates, @cost_entry.project) %> - <%= hidden_field_tag :cost_value, unitless_currency_number(@cost_entry.real_costs), id: 'cost_entry_cost_value' %> - - - <%= number_to_currency(@cost_entry.real_costs) %> - - - - <% else %> +
+ + <% if User.current.allowed_to?(:view_cost_rates, @cost_entry.project) %> + + + <%= number_to_currency(@cost_entry.real_costs) %> + + <%= render partial: '/budgets/items/budget_override_cost_form', + locals: { + field_name: "cost_entry[overridden_costs]", + cost_value: unitless_currency_number(@cost_entry.real_costs), + } + %> + + + <% else %> + <%= f.text_field :overridden_costs, value: @cost_entry.overridden_costs ? unitless_currency_number(@cost_entry.overridden_costs).strip : '', placeholder: t(:label_example_placeholder, decimal: unitless_currency_number(1000.50)), @@ -140,13 +142,12 @@ See COPYRIGHT and LICENSE files for more details. id: 'cost_entry_cost_value', container_class: '-middle', size: 7 %> - -
-

<%= t(:help_override_rate) %>

-
- <% end %> -
- + +
+

<%= t(:help_override_rate) %>

+
+ <% end %> +
<%= f.text_field :comments, size: 100, container_class: '-wide' %> diff --git a/modules/costs/spec/features/cost_entries/add_cost_entry_spec.rb b/modules/costs/spec/features/cost_entries/add_cost_entry_spec.rb index 2adee36d2e..5502b6cc52 100644 --- a/modules/costs/spec/features/cost_entries/add_cost_entry_spec.rb +++ b/modules/costs/spec/features/cost_entries/add_cost_entry_spec.rb @@ -93,7 +93,7 @@ describe 'Work Package cost fields', type: :feature, js: true do # Override costs find('#cost_entry_costs').click SeleniumHubWaiter.wait - fill_in 'cost_entry_costs_edit', with: '15.52' + fill_in 'cost_entry_overridden_costs', with: '15.52' click_on 'Save' @@ -130,7 +130,7 @@ describe 'Work Package cost fields', type: :feature, js: true do # Override costs find('#cost_entry_costs').click SeleniumHubWaiter.wait - fill_in 'cost_entry_costs_edit', with: '1.350,25' + fill_in 'cost_entry_overridden_costs', with: '1.350,25' click_on I18n.t(:button_save) @@ -152,7 +152,7 @@ describe 'Work Package cost fields', type: :feature, js: true do # Update the costs in german locale SeleniumHubWaiter.wait - fill_in 'cost_entry_costs_edit', with: '55.000,55' + fill_in 'cost_entry_overridden_costs', with: '55.000,55' click_on I18n.t(:button_save) expect(page).to have_selector('#cost_entry_costs', text: '55.000,55 EUR') diff --git a/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb b/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb index d18f1ead64..6aeb266552 100644 --- a/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb +++ b/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb @@ -72,7 +72,7 @@ module LdapGroups if call.success? Rails.logger.info("[LDAP groups] User '#{call.result.login}' created") else - Rails.logger.error("[LDAP groups] User '#{user}' could not be created: #{call.message}") + Rails.logger.error("[LDAP groups] User '#{call.result&.login}' could not be created: #{call.message}") end end diff --git a/spec/features/projects/template_spec.rb b/spec/features/projects/template_spec.rb index 7620ac51cc..40298ffe76 100644 --- a/spec/features/projects/template_spec.rb +++ b/spec/features/projects/template_spec.rb @@ -100,7 +100,9 @@ describe 'Project templates', type: :feature, js: true do # It does not show the copy meta flags expect(page).to have_no_selector('[data-qa-field-name="copyMembers"]') - expect(page).to have_no_selector('[data-qa-field-name="sendNotifications"]') + + # But shows the send notifications field + expect(page).to have_selector('[data-qa-field-name="sendNotifications"]') # Update status to off track status_field.select_option 'Off track' diff --git a/spec/models/queries/notifications/notification_query_spec.rb b/spec/models/queries/notifications/notification_query_spec.rb index e7dab0b5e4..acea32fca9 100644 --- a/spec/models/queries/notifications/notification_query_spec.rb +++ b/spec/models/queries/notifications/notification_query_spec.rb @@ -29,10 +29,17 @@ require 'spec_helper' describe Queries::Notifications::NotificationQuery, type: :model do - shared_let(:recipient) { FactoryBot.create :user } + shared_let(:project) { FactoryBot.create :project } + + shared_let(:recipient) { FactoryBot.create :user, member_in_project: project, member_with_permissions: %i[view_work_packages] } + + shared_let(:work_package) { FactoryBot.create :work_package, project: project } + shared_let(:notification) { FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package } let(:instance) { described_class.new(user: recipient) } - let(:base_scope) { Notification.recipient(recipient) } + let(:base_scope) { Notification.visible(recipient).recipient(recipient) } + + current_user { recipient } context 'without a filter' do describe '#results' do @@ -49,9 +56,9 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = base_scope.where("notifications.read_ian IN ('t')").order(id: :desc) + expected = base_scope.merge(Notification.where("notifications.read_ian IN ('t')").order(id: :desc)).to_sql - expect(instance.results.to_sql).to eql expected.to_sql + expect(instance.results.to_sql).to eql expected end end @@ -114,11 +121,7 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = <<~SQL.squish - SELECT "notifications".* FROM "notifications" - WHERE "notifications"."recipient_id" = #{recipient.id} - ORDER BY "notifications"."read_ian" DESC, "notifications"."id" DESC - SQL + expected = base_scope.merge(Notification.order(read_ian: :desc, id: :desc)).to_sql expect(instance.results.to_sql).to eql expected end @@ -132,11 +135,7 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = <<~SQL.squish - SELECT "notifications".* FROM "notifications" - WHERE "notifications"."recipient_id" = #{recipient.id} - ORDER BY "notifications"."reason" DESC, "notifications"."id" DESC - SQL + expected = base_scope.merge(Notification.order(reason: :desc, id: :desc)).to_sql expect(instance.results.to_sql).to eql expected end @@ -170,12 +169,11 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = <<~SQL.squish - SELECT "notifications"."reason", COUNT(*) FROM "notifications" - WHERE "notifications"."recipient_id" = #{recipient.id} - GROUP BY "notifications"."reason" - ORDER BY "notifications"."reason" ASC - SQL + scope = Notification + .group(:reason) + .order(reason: :asc) + .select(:reason, Arel.sql('COUNT(*)')) + expected = base_scope.merge(scope).to_sql expect(instance.groups.to_sql).to eql expected end @@ -189,12 +187,11 @@ describe Queries::Notifications::NotificationQuery, type: :model do describe '#results' do it 'is the same as handwriting the query' do - expected = <<~SQL.squish - SELECT "notifications"."project_id", COUNT(*) FROM "notifications" - WHERE "notifications"."recipient_id" = #{recipient.id} - GROUP BY "notifications"."project_id" - ORDER BY "notifications"."project_id" ASC - SQL + scope = Notification + .group(:project_id) + .order(project_id: :asc) + .select(:project_id, Arel.sql('COUNT(*)')) + expected = base_scope.merge(scope).to_sql expect(instance.groups.to_sql).to eql expected end diff --git a/spec/requests/api/v3/notifications/bulk_read_ian_resource_spec.rb b/spec/requests/api/v3/notifications/bulk_read_ian_resource_spec.rb index a1837ae444..77ae690520 100644 --- a/spec/requests/api/v3/notifications/bulk_read_ian_resource_spec.rb +++ b/spec/requests/api/v3/notifications/bulk_read_ian_resource_spec.rb @@ -35,11 +35,16 @@ describe ::API::V3::Notifications::NotificationsAPI, content_type: :json do include API::V3::Utilities::PathHelper - shared_let(:recipient) { FactoryBot.create :user } + shared_let(:project) { FactoryBot.create :project } + + shared_let(:recipient) { FactoryBot.create :user, member_in_project: project, member_with_permissions: %i[view_work_packages] } shared_let(:other_recipient) { FactoryBot.create :user } - shared_let(:notification1) { FactoryBot.create :notification, recipient: recipient } - shared_let(:notification2) { FactoryBot.create :notification, recipient: recipient } - shared_let(:notification3) { FactoryBot.create :notification, recipient: recipient } + + shared_let(:work_package) { FactoryBot.create :work_package, project: project } + + shared_let(:notification1) { FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package } + shared_let(:notification2) { FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package } + shared_let(:notification3) { FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package } shared_let(:other_user_notification) { FactoryBot.create :notification, recipient: other_recipient } let(:filters) { nil } diff --git a/spec/requests/api/v3/notifications/bulk_unread_ian_resource_spec.rb b/spec/requests/api/v3/notifications/bulk_unread_ian_resource_spec.rb index e8fb036494..cc93b286b4 100644 --- a/spec/requests/api/v3/notifications/bulk_unread_ian_resource_spec.rb +++ b/spec/requests/api/v3/notifications/bulk_unread_ian_resource_spec.rb @@ -35,12 +35,27 @@ describe ::API::V3::Notifications::NotificationsAPI, content_type: :json do include API::V3::Utilities::PathHelper - shared_let(:recipient) { FactoryBot.create :user } + shared_let(:project) { FactoryBot.create :project } + shared_let(:work_package) { FactoryBot.create :work_package, project: project } + + shared_let(:recipient) { FactoryBot.create :user, member_in_project: project, member_with_permissions: %i[view_work_packages] } shared_let(:other_recipient) { FactoryBot.create :user } - shared_let(:notification1) { FactoryBot.create :notification, recipient: recipient, read_ian: true } - shared_let(:notification2) { FactoryBot.create :notification, recipient: recipient, read_ian: true } - shared_let(:notification3) { FactoryBot.create :notification, recipient: recipient, read_ian: true } - shared_let(:other_user_notification) { FactoryBot.create :notification, recipient: other_recipient, read_ian: true } + shared_let(:notification1) do + FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package, read_ian: true + end + shared_let(:notification2) do + FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package, read_ian: true + end + shared_let(:notification3) do + FactoryBot.create :notification, recipient: recipient, project: project, resource: work_package, read_ian: true + end + shared_let(:other_user_notification) do + FactoryBot.create :notification, + recipient: other_recipient, + read_ian: true, + project: project, + resource: work_package + end let(:filters) { nil } @@ -86,10 +101,10 @@ describe ::API::V3::Notifications::NotificationsAPI, end it 'sets the current users`s notifications matching the filter to read' do - expect(::Notification.where(id: [notification1.id, notification2.id]).pluck(:read_ian)) + expect(::Notification.where(id: [notification1.id, notification2.id]).order(id: :asc).pluck(:read_ian)) .to all(be_falsey) - expect(::Notification.where(id: [other_user_notification, notification3.id]).pluck(:read_ian)) + expect(::Notification.where(id: [other_user_notification.id, notification3.id]).order(id: :asc).pluck(:read_ian)) .to all(be_truthy) end end