Skip to content

[base + modules] Issue #670 - Remove side effects in manage_schema functions #671

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[base + modules] Issue #670 - Remove side effects in manage_schema functions #671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09e3eaf
Issue #670 - Remove side effects in manage_schema functions
robvandenbogaard Jul 7, 2020
c98e566
Issue #670 - Cleanup unused variables
robvandenbogaard Jul 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions modules/mod_ginger_banner/mod_ginger_banner.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@
manage_schema/2
]).

manage_schema(_Version, Context) ->
Datamodel = #datamodel{
manage_schema(_Version, _Context) ->
#datamodel{
resources = [
{message_banner, text, [
{title, "Banner"},
{is_published, false}
]}
]
},
z_datamodel:manage(?MODULE, Datamodel, Context).
}.
8 changes: 3 additions & 5 deletions modules/mod_ginger_base/mod_ginger_base.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ init(Context) ->
z_pivot_rsc:define_custom_pivot(ginger_search, [{is_unfindable, "boolean not null default false"}], Context).

%% @doc When ACL is enabled, create a default user in the editors group
manage_schema(_Version, Context) ->
Datamodel = #datamodel{
manage_schema(_Version, _Context) ->
#datamodel{
categories=[
{agenda, query, [
{title, {trans, [
Expand Down Expand Up @@ -168,9 +168,7 @@ manage_schema(_Version, Context) ->
]}
]}
]
},
z_datamodel:manage(?MODULE, Datamodel, Context),
schema:create_identity_if_not_exists(editor_dev, "redacteur", "redacteur", Context).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It went nowhere, as I think it is not used / we should not use it (here) and rethink it when we need it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use manage_data/2, it is called after manage_schema/2

manage_data(_Version, Context) ->
    %% Whatever data needs to be installed after the datamodel
    %% has been installed.
    ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal to remove the automatic creation of editor_dev. If we need one we should provide it in fixtures.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've never used it, maybe @loetie has

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also an manage_data/2 function. That is called after the manage_schema/2 has been handled.

See https://test.zotonic.com/page/1353/modules

manage_data(_Version, Context) ->
    %% Whatever data needs to be installed after the datamodel
    %% has been installed.
    ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW schema is a dangerous name for a module. I can imagine that other projects will also try to use this generic name. Better prefix it with ginger_, as it is a Ginger-only module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I completely agree, that struck me too.

}.

%% @doc Users without access to the admin should not be able to view unpublished
%% resources
Expand Down
7 changes: 3 additions & 4 deletions modules/mod_ginger_embed/mod_ginger_embed.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

-include("zotonic.hrl").

manage_schema(_Version, Context) ->
Datamodel = #datamodel{
manage_schema(_Version, _Context) ->
#datamodel{
categories=[
{ginger_embed, media, [
{title, {trans, [
Expand All @@ -30,8 +30,7 @@ manage_schema(_Version, Context) ->
{language, [en, nl]}
]}
]
},
z_datamodel:manage(?MODULE, Datamodel, Context).
}.

%% @doc Render embed template in case of <ginger-embed> element
-spec observe_media_viewer(#media_viewer{}, #context{}) -> {ok, binary()} | undefined.
Expand Down
8 changes: 3 additions & 5 deletions modules/mod_ginger_message/mod_ginger_message.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
manage_schema/2
]).

manage_schema(install, Context) ->
Datamodel = #datamodel{
manage_schema(install, _Context) ->
#datamodel{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context -> _Context (unused)

categories=[
{message, undefined, [
{is_unfindable, 1},
Expand Down Expand Up @@ -52,6 +52,4 @@ manage_schema(install, Context) ->
{message, category}
]}
]
},
z_datamodel:manage(?MODULE, Datamodel, Context),
ok.
}.
26 changes: 13 additions & 13 deletions modules/mod_ginger_rdf/mod_ginger_rdf.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,8 @@
-record(state, {context}).

manage_schema(_, Context) ->
Datamodel = #datamodel{
categories=[
{rdf, meta, [{title, <<"RDF resource">>}]}
],
resources=[
{rdf_content_group, content_group, [
{title, <<"RDF resources">>}
]}
]
},
z_datamodel:manage(?MODULE, Datamodel, Context),

%% TODO: Legacy code ahead! This function must not have side effects;
%% it should only return the data model.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look into this case to see if it is still necessary to keep this here, or if it could be in a better place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use manage_data/2 to update/install data after the schema has been installed.

%% Update some predicates so they can refer to category RDF, too
case m_rsc:uri_lookup("http://xmlns.com/foaf/0.1/depiction", Context) of
undefined -> noop;
Expand All @@ -58,7 +48,17 @@ manage_schema(_, Context) ->
Context
)
end,
ok.
#datamodel{
categories=[
{rdf, meta, [
{title, <<"RDF resource">>}
]}
],
resources=[
{rdf_content_group, content_group, [
{title, <<"RDF resources">>}
]}
]}.

%% @doc Ask observers to provide subject links from the resource and object
%% links to the resource
Expand Down
5 changes: 2 additions & 3 deletions modules/mod_ginger_remark/mod_ginger_remark.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
]).

manage_schema(_Version, Context) ->
Datamodel = #datamodel{
#datamodel{
categories=[
{remark, text, [
{title, {trans, [{nl, <<"Reactie">>},
Expand Down Expand Up @@ -54,8 +54,7 @@ manage_schema(_Version, Context) ->
{is_owner, true}
]}
]}
]},
z_datamodel:manage(?MODULE, Datamodel, Context).
]}.

is_owner(Id, #context{user_id=undefined, session_id=SessionId} = Context) ->
case m_rsc:p_no_acl(Id, session_owner, Context) of
Expand Down
16 changes: 9 additions & 7 deletions modules/mod_ginger_tagger/mod_ginger_tagger.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,21 @@
event/2
]).

manage_schema(install, Context) ->
Datamodel = #datamodel{
manage_schema(install, _Context) ->
#datamodel{
categories = [
{rfid_device, undefined, [{title, <<"RFID-scanner">>}]}
{rfid_device, undefined, [
{title, <<"RFID-scanner">>}
]}
],
predicates = [
{located_at, [{title, <<"Location">>}], [
{located_at, [
{title, <<"Location">>}
], [
{rfid_device, location}
]}
]
},
z_datamodel:manage(?MODULE, Datamodel, Context),
ok.
}.

%% @doc Link users to media in which they are depicted.
-spec observe_tagger_action(#tagger_action{}, #context{}) -> ok.
Expand Down