The status of a mapping can now be 'overridden' because it is a) an override of a default mapping or b) an override of a per content type mapping.

We need to resolve this naming conflict by avoiding the term override when creating per node type mappings.

CommentFileSizeAuthor
#7 557500-1_name_conflict.patch8.88 KBaron novak
#3 557500_name_conflict.patch8.17 KBaron novak

Comments

alex_b’s picture

alex_b commented

This naming conflict needs to be not only solved for the UI level but for the code level as well.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented

I'd say when we're talking about per-content-type and per-node mapping, this is actually the scope of the mapping.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
Status: Needs work » Needs review
StatusFileSize
new 557500_name_conflict.patch8.17 KB

tests are 100%
new terminology: scope for talking about the whole stuff, restrict when we want per-node mapping.
Both UI and code are fixed.

alex_b’s picture

alex_b commented
Status: Needs review » Needs work

I'd tweak this slightly. Why don't we do this:

--

Scope: This mapping is the standard content-type mapping. Any changes will affect all feeds with the same content-type. [Change to per node type].

Status: default (This mapping is defined in code).

--

Scope: This mapping is specific to this feed node. [Use standard content-type mapping]

Status: overridden (There is a default mapping defined in code but it is overridden in the database).

--

Status: normal (This mapping is defined in the database).

--

Rename _feedapi_mapper_is_overridden() to _feedapi_mapper_is_per_content_type().

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented

"Rename _feedapi_mapper_is_overridden() to _feedapi_mapper_is_per_content_type()."
_feedapi_mapper_is_overridden() even does not exists anymore. It is called _feedapi_mapper_scope() in #3
As i see you like "scope" in general, so that function name should be okay if we use scope in the UI

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented

"[Change to per node type]" - per node type is misleading, because it's rather similar to content type. People usually mix "node type" and "content type" phrases, so i'd say [Change to per node] instead.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
Status: Needs work » Needs review
StatusFileSize
new 557500-1_name_conflict.patch8.88 KB

UI text is improved a little bit according to #4 (but without the "node type" phrase)

alex_b’s picture

alex_b commented
Status: Needs review » Fixed

Committed with some text tweaks and UI tweaks. Also updated tests. Thank you.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.