Skip to content
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.

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

Add documentation for logic customization #9325

Merged
merged 11 commits into from
Jun 14, 2022

Conversation

sinaeftekhar
Copy link
Contributor

@sinaeftekhar sinaeftekhar commented May 18, 2022

🎩 What? Why?

Currently, decidim documentation is lacking logic customization. This section is added to the "Getting started section.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

@sinaeftekhar sinaeftekhar marked this pull request as draft May 18, 2022 14:05
@sinaeftekhar sinaeftekhar marked this pull request as ready for review May 19, 2022 08:43
Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Overall this looks great but I think it needs some refining. I've left some suggestions below.

Also, I noticed we already have a "Customize" section in the documentation, e.g. how to customize the views:
https://docs.decidim.org/en/customize/views/

Or at GitHub:
https://github.com/decidim/decidim/tree/develop/docs/modules/customize/pages

I think this would be a natural place for this piece of documentation, we can name it e.g. customize/logic, so it would become available at this URL after these changes:
https://docs.decidim.org/en/customize/logic/

As we discussed in another issue, the documentation might now be perfectly structured right now, but for this particular addition, let's first add it where it naturally belongs and later fix the issues with the navigational structure as a whole.

docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
docs/modules/install/pages/customization.adoc Outdated Show resolved Hide resolved
@sinaeftekhar
Copy link
Contributor Author

@ahukkanen
All review issues addressed.

@sinaeftekhar sinaeftekhar requested a review from ahukkanen May 20, 2022 14:24
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Great addition and contribution @sinaeftekhar!
I have some feedback, can you give it a look please? Thanks

docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Few more improvement suggestions + please review one of the previous comments (I marked it as unresolved above).

docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
docs/modules/customize/pages/logic.adoc Outdated Show resolved Hide resolved
Changes to the content of the page was done according to reviews
suggestion.
@ahukkanen
Copy link
Contributor

@andreslucena This is good for me, so feel free to merge once you are happy with it.

Regarding your previous comment.

It feels like the idea of this page is really similar to https://github.com/decidim/decidim/blob/develop/docs/modules/customize/pages/code.adoc, but with much more detail. So should we delete that and use this? Maybe adding the real world examples and also the modules part?

We have gone through several iterations of doing customizations within an instance and figured the concern way is probably the cleanest way to achieve this and keep track of what has changed.

The decorator approach in the linked example doesn't look too bad but we prefer the concern way.

As a real life example, I think the "Simple Proposal" module we've built is quite a good real life example (we use it with almost all of the smaller clients):
https://github.com/mainio/decidim-module-simple_proposal

Of course id doesn't demonstrate how to do the customizations directly from the application code but there's nothing stopping you from applying similar customizations from the application itself.

@andreslucena
Copy link
Member

figured the concern way is probably the cleanest way to achieve this and keep track of what has changed

To me too, of course.

I was only talking about having two similar but different sections in the documentation navigation for modifying Ruby code. But I actually don't have it clear on how to do it, so when I have a proposal I'll make the PR.

For sure, this is a clear improvement on what we have now!!

Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@andreslucena andreslucena merged commit 53b8e66 into decidim:develop Jun 14, 2022
@ahukkanen ahukkanen deleted the doc/add_customization branch June 14, 2022 09:03
andreslucena pushed a commit that referenced this pull request Jul 6, 2022
* Add documentation page

* Add Customization section

* Edit documentation for the customization section

* Resolve comments by the reviewer

* Change the location of the files

* Edit page title

* Edit reviewer's comments

- Fix indentation.
- Update refered source code
- Edit content

* Edit content as reviewers' request

Changes to the content of the page was done according to reviews
suggestion.

* resolve reviewer's commented issue
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
* Add documentation page

* Add Customization section

* Edit documentation for the customization section

* Resolve comments by the reviewer

* Change the location of the files

* Edit page title

* Edit reviewer's comments

- Fix indentation.
- Update refered source code
- Edit content

* Edit content as reviewers' request

Changes to the content of the page was done according to reviews
suggestion.

* resolve reviewer's commented issue
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants