Skip to content

Commit

Permalink
Maintenance: Renamed confusing fields in EmailAddresses table
Browse files Browse the repository at this point in the history
The EmailAddress model is used to store email addresses used by the system.
It was using realname: field as a name while the rest of the system uses name: for, well, naming objects.
This commit renames email_addresses.realname to email_addresses.name for the sake of consistency.
  • Loading branch information
mantas committed Apr 11, 2023
1 parent fc2491a commit e1cf198
Show file tree
Hide file tree
Showing 30 changed files with 75 additions and 63 deletions.
10 changes: 5 additions & 5 deletions app/assets/javascripts/app/models/email_address.coffee
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
class App.EmailAddress extends App.Model
@configure 'EmailAddress', 'realname', 'email', 'channel_id', 'note', 'active', 'updated_at'
@configure 'EmailAddress', 'name', 'email', 'channel_id', 'note', 'active', 'updated_at'
@extend Spine.Model.Ajax
@url: @apiPath + '/email_addresses'

displayName: ->
if @realname
if @name
# Do not use App.Utils.buildEmailAddress here because we don't build an email address and the
# quoting would confuse sorting in the GUI.
return "#{@realname} <#{@email}>"
return "#{@name} <#{@email}>"
@email

@filterChannel: (options, type, params) ->
Expand All @@ -29,13 +29,13 @@ class App.EmailAddress extends App.Model
)

@configure_attributes = [
{ name: 'realname', display: __('Display name'), tag: 'input', type: 'text', limit: 250, null: false },
{ name: 'name', display: __('Display name'), tag: 'input', type: 'text', limit: 250, null: false },
{ name: 'email', display: __('Email'), tag: 'input', type: 'email', limit: 250, null: false },
{ name: 'channel_id', display: __('Channel'), tag: 'select', multiple: false, null: true, relation: 'Channel', nulloption: true, filter: @filterChannel, do_not_log: true },
{ name: 'note', display: __('Note'), tag: 'textarea', note: __('Notes are visible to agents only, never to customers.'), limit: 250, null: true },
{ name: 'updated_at', display: __('Updated'), tag: 'datetime', readonly: 1 },
{ name: 'active', display: __('Active'), tag: 'active', readonly: 1 },
]
@configure_overview = [
'realname', 'email'
'name', 'email'
]
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<div class="action-block">
<ul>
<% for email_address in @accounts_fixed: %>
<li><%= email_address.realname %> &lt;<%= email_address.email %>&gt;
<li><%= email_address.name %> &lt;<%= email_address.email %>&gt;
<% end %>
</ul>
</div>
Expand All @@ -25,7 +25,7 @@
<% for email_address in @not_used_email_addresses: %>
<li class="list-item" data-id="<%= email_address.id %>">
<div class="list-item-name">
<a href="#" class="js-emailAddressEdit"><%= email_address.realname %> &lt;<%= email_address.email %>&gt;</a>
<a href="#" class="js-emailAddressEdit"><%= email_address.name %> &lt;<%= email_address.email %>&gt;</a>
</div>
<div class="list-item-delete js-emailAddressDelete">
<%- @Icon('diagonal-cross') %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<p class="text-center"><%- @T('Your Zammad has the following email address:') %></p>
<% if @addresses: %>
<% for address in @addresses: %>
<p><%= address.realname %> &lt;<%= address.email %>&gt;</p>
<p><%= address.name %> &lt;<%= address.email %>&gt;</p>
<% end %>
<% end %>
<p class="text-center"><%- @T('If you want to use additional email addresses, you can configure them later.') %></p>
Expand All @@ -18,4 +18,4 @@
</div>
</div>
</form>
</div>
</div>
2 changes: 1 addition & 1 deletion app/assets/javascripts/app/views/google/list.jst.eco
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<% for email_address in @not_used_email_addresses: %>
<li class="list-item" data-id="<%= email_address.id %>">
<div class="list-item-name">
<a href="#" class="js-emailAddressEdit"><%= email_address.realname %> &lt;<%= email_address.email %>&gt;</a>
<a href="#" class="js-emailAddressEdit"><%= email_address.name %> &lt;<%= email_address.email %>&gt;</a>
</div>
<div class="list-item-delete js-emailAddressDelete">
<%- @Icon('diagonal-cross') %>
Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/app/views/microsoft365/list.jst.eco
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<% for email_address in @not_used_email_addresses: %>
<li class="list-item" data-id="<%= email_address.id %>">
<div class="list-item-name">
<a href="#" class="js-emailAddressEdit"><%= email_address.realname %> &lt;<%= email_address.email %>&gt;</a>
<a href="#" class="js-emailAddressEdit"><%= email_address.name %> &lt;<%= email_address.email %>&gt;</a>
</div>
<div class="list-item-delete js-emailAddressDelete">
<%- @Icon('diagonal-cross') %>
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/channels_email_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,14 @@ def verify

if address
address.update!(
realname: params[:meta][:realname],
name: params[:meta][:realname],
email: email,
active: true,
channel_id: channel.id,
)
else
EmailAddress.create(
realname: params[:meta][:realname],
name: params[:meta][:realname],
email: email,
active: true,
channel_id: channel.id,
Expand Down
14 changes: 7 additions & 7 deletions app/controllers/email_addresses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class EmailAddressesController < ApplicationController
Example:
{
"id":1,
"realname":"some realname",
"name":"some realname",
"email":"[email protected]",
"updated_at":"2012-09-14T17:51:53Z",
"created_at":"2012-09-14T17:51:53Z",
Expand All @@ -30,12 +30,12 @@ class EmailAddressesController < ApplicationController
[
{
"id": 1,
"realname":"some realname1",
"name":"some realname1",
...
},
{
"id": 2,
"realname":"some realname2",
"name":"some realname2",
...
}
]
Expand Down Expand Up @@ -77,7 +77,7 @@ def show
Payload:
{
"realname":"some realname",
"name":"some realname",
"email":"[email protected]",
"note": "",
"active":true,
Expand All @@ -86,7 +86,7 @@ def show
Response:
{
"id": 1,
"realname":"some realname",
"name":"some realname",
"email":"[email protected]",
...
}
Expand All @@ -107,7 +107,7 @@ def create
Payload:
{
"realname":"some realname",
"name":"some realname",
"email":"[email protected]",
"note": "",
"active":true,
Expand All @@ -116,7 +116,7 @@ def create
Response:
{
"id": 1,
"realname":"some realname",
"name":"some realname",
"email":"[email protected]",
...
}
Expand Down
2 changes: 1 addition & 1 deletion app/graphql/gql/types/group_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def email_address

email_address = @object.email_address

{ name: email_address.realname, email_address: email_address.email }
{ name: email_address.name, email_address: email_address.email }
end
end
end
4 changes: 2 additions & 2 deletions app/models/email_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ class EmailAddress < ApplicationModel

has_many :groups, after_add: :cache_update, after_remove: :cache_update
belongs_to :channel, optional: true
validates :realname, presence: true
validates :email, presence: true
validates :name, presence: true
validates :email, presence: true

before_validation :check_email
before_create :check_if_channel_exists_set_inactive
Expand Down
4 changes: 2 additions & 2 deletions app/models/ticket/article/adds_metadata_email.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ def recipient_name(email_address)
case Setting.get('ticket_define_email_from')
when 'AgentNameSystemAddressName'
separator = Setting.get('ticket_define_email_from_separator')
return "#{created_by.firstname} #{created_by.lastname} #{separator} #{email_address.realname}"
return "#{created_by.firstname} #{created_by.lastname} #{separator} #{email_address.name}"
when 'AgentName'
return "#{created_by.firstname} #{created_by.lastname}"
end
end

email_address.realname
email_address.name
end

def metadata_email_process_from
Expand Down
2 changes: 1 addition & 1 deletion contrib/auto_wizard_browser_test.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
{
"id": 1,
"channel_id": 3,
"realname": "Some Realname",
"name": "Some Realname",
"email": "[email protected]"
}
],
Expand Down
2 changes: 1 addition & 1 deletion contrib/auto_wizard_example.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
{
"id": 1,
"channel_id": 10,
"realname": "Zammad Demo System",
"name": "Zammad Demo System",
"email": "zammad_demo@localhost"
}
],
Expand Down
2 changes: 1 addition & 1 deletion contrib/auto_wizard_test.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
{
"id": 1,
"channel_id": 1,
"realname": "Zammad Helpdesk",
"name": "Zammad Helpdesk",
"email": "zammad@localhost"
}
],
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/20120101000001_create_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def up

create_table :email_addresses do |t|
t.integer :channel_id, null: true
t.string :realname, limit: 250, null: false
t.string :name, limit: 250, null: false
t.string :email, limit: 255, null: false
t.boolean :active, null: false, default: true
t.string :note, limit: 250, null: true
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20230410202420_rename_email_realname.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/

class RenameEmailRealname < ActiveRecord::Migration[6.1]
def change
# return if it's a new setup
return if !Setting.exists?(name: 'system_init_done')

rename_column :email_addresses, :realname, :name

EmailAddress.reset_column_information
end
end
10 changes: 5 additions & 5 deletions lib/external_credential/google.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ def self.link_account(_request_token, params)

email_addresses = user_aliases(response)
email_addresses.unshift({
realname: "#{Setting.get('product_name')} Support",
email: user_data[:email],
name: "#{Setting.get('product_name')} Support",
email: user_data[:email],
})

email_addresses.each do |email|
Expand All @@ -145,7 +145,7 @@ def self.link_account(_request_token, params)
email_addresses.each do |user_alias|
EmailAddress.create!(
channel_id: channel.id,
realname: user_alias[:realname],
name: user_alias[:name],
email: user_alias[:email],
active: true,
created_by_id: 1,
Expand Down Expand Up @@ -262,8 +262,8 @@ def self.user_aliases(token)
next if row['verificationStatus'] != 'accepted'

aliases.push({
realname: row['displayName'],
email: row['sendAsEmail'],
name: row['displayName'],
email: row['sendAsEmail'],
})
end

Expand Down
6 changes: 3 additions & 3 deletions lib/external_credential/microsoft365.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ def self.link_account(_request_token, params)

email_addresses = [
{
realname: "#{Setting.get('product_name')} Support",
email: user_data[:preferred_username],
name: "#{Setting.get('product_name')} Support",
email: user_data[:preferred_username],
},
]

Expand All @@ -154,7 +154,7 @@ def self.link_account(_request_token, params)
email_addresses.each do |user_alias|
EmailAddress.create!(
channel_id: channel.id,
realname: user_alias[:realname],
name: user_alias[:name],
email: user_alias[:email],
active: true,
created_by_id: 1,
Expand Down
6 changes: 3 additions & 3 deletions public/assets/tests/qunit/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -643,14 +643,14 @@ QUnit.test('table test 4', assert => {
App.EmailAddress.refresh( [
{
id: 55,
realname: 'realname 55',
name: 'realname 55',
email: 'email 55',
active: true,
created_at: '2014-06-10T11:17:34.000Z',
},
{
id: 56,
realname: 'realname 56',
name: 'realname 56',
email: 'email 56',
active: true,
created_at: '2014-06-10T11:17:34.000Z',
Expand Down Expand Up @@ -697,7 +697,7 @@ QUnit.test('table test 4', assert => {
new App.ControllerTable({
el: el,
model: App.EmailAddress,
objects: App.EmailAddress.search({sortBy:'realname', order: 'ASC'}),
objects: App.EmailAddress.search({sortBy:'name', order: 'ASC'}),
callbackHeader: [callbackHeader],
callbackAttributes: {
'some name': [ callbackAttributes ]
Expand Down
8 changes: 4 additions & 4 deletions spec/factories/email_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
FactoryBot.define do
factory :email_address do
association :channel, factory: :email_channel
sequence(:email) { |n| "zammad#{n}@localhost.com" }
sequence(:realname) { |n| "zammad#{n}" }
created_by_id { 1 }
updated_by_id { 1 }
sequence(:email) { |n| "zammad#{n}@localhost.com" }
sequence(:name) { |n| "zammad#{n}" }
created_by_id { 1 }
updated_by_id { 1 }
end
end
4 changes: 2 additions & 2 deletions spec/graphql/gql/types/group_type_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

it 'has email address and name' do
expect(instance.email_address)
.to include(name: group.email_address.realname, email_address: group.email_address.email)
.to include(name: group.email_address.name, email_address: group.email_address.email)
end
end

Expand Down Expand Up @@ -62,7 +62,7 @@
'group' => include(
'name' => group.name,
'emailAddress' => include(
'name' => group.email_address.realname,
'name' => group.email_address.name,
'emailAddress' => group.email_address.email,
)
),
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/auto_wizard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
EmailAddresses: [
{
channel_id: channel.id,
realname: 'John Doe',
name: 'John Doe',
email: '[email protected]',
}
],
Expand All @@ -141,7 +141,7 @@
it 'creates an email address with the given attributes' do
expect { described_class.setup }
.to change(EmailAddress, :count)
.and change { EmailAddress.last&.realname }.to('John Doe')
.and change { EmailAddress.last&.name }.to('John Doe')
.and change { EmailAddress.last&.email }.to('[email protected]')
.and change { EmailAddress.last&.channel }.to(channel)
end
Expand All @@ -154,7 +154,7 @@
{
id: email_address.id,
channel_id: new_channel.id,
realname: 'John Doe',
name: 'John Doe',
email: '[email protected]',
}
],
Expand All @@ -167,7 +167,7 @@
it 'updates the specified email address with the given attributes' do
expect { described_class.setup }
.to not_change(EmailAddress, :count)
.and change { email_address.reload.realname }.to('John Doe')
.and change { email_address.reload.name }.to('John Doe')
.and change { email_address.reload.email }.to('[email protected]')
.and change { email_address.reload.channel }.to(new_channel)
end
Expand Down
Loading

0 comments on commit e1cf198

Please sign in to comment.