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

organisations: favicon and platform name #768

Merged
merged 1 commit into from
Mar 7, 2022

Conversation

Garfield-fr
Copy link
Contributor

@Garfield-fr Garfield-fr commented Feb 8, 2022

Co-Authored-by: Bertrand Zuchuat bertrand.zuchuat@rero.ch

How to test

  • Upload favicon.ico to organisation files.
  • Check the organisation favicon and name in browser tab.

favicon_file

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@Garfield-fr Garfield-fr force-pushed the zub-favicon-viewname branch 5 times, most recently from 78e2a6f to 2b693ef Compare February 8, 2022 13:38
@Garfield-fr Garfield-fr self-assigned this Feb 8, 2022
@Garfield-fr Garfield-fr force-pushed the zub-favicon-viewname branch 2 times, most recently from 217d9be to 35086dd Compare February 8, 2022 14:11
sonar/config.py Outdated
@@ -58,6 +58,9 @@ def _(x):
return x


# Sonar global view name
SONAR_GLOBAL_VIEW_NAME = 'global'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using SONAR_APP_DEFAULT_ORGANISATION?

sonar/ext.py Outdated
@app.template_filter()
def organisation_view(pid):
"""Get current organisation."""
return OrganisationRecord.get_or_create(pid) if pid else {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks not safe. It can in some case create a new organisation, no?

sonar/ext.py Outdated
lambda d: d['mimetype'] == 'image/x-icon', files))
if favicon:
file = favicon[0]['key']
return f'/organisations/{key}/files/{file}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use url_for.

sonar/ext.py Outdated
key = org.get('pid')
files = org.get('_files', [])
favicon = list(filter(
lambda d: d['mimetype'] == 'image/x-icon', files))
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be better to introduce a new type of file: favicon

@Garfield-fr Garfield-fr force-pushed the zub-favicon-viewname branch 5 times, most recently from cd31206 to 3214bff Compare February 10, 2022 09:49
@pronguen
Copy link
Contributor

pronguen commented Mar 1, 2022

  • NPR: the organisation name is not added in the title of the page (does not appear in the tab), at least on the view homepage.
  • NPR: favicon does not work
  • NPR: setting an organisation to dedicated creates sometimes a bug on its homepage
    1. see https://sonardev.test.rero.ch/hepfr/ is not working
    2. add a favicon to the org (pid:hepfr) > the homepage is working
    3. remove the favicon > the homepage is working
    4. edit and save the org (without any changes) > the homepage is not working

@Garfield-fr Garfield-fr force-pushed the zub-favicon-viewname branch 3 times, most recently from c4071ea to 31ac72b Compare March 2, 2022 13:15
Copy link
Contributor

@pronguen pronguen left a comment

Choose a reason for hiding this comment

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

Nickel 👌

* Adds the display of the custom favicon for current organisation.
* Adds the organization's platform name to the browser tab.
* Closes rero#705.

Co-Authored-by: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
@Garfield-fr Garfield-fr merged commit 47b402e into rero:staging Mar 7, 2022
@Garfield-fr Garfield-fr deleted the zub-favicon-viewname branch March 8, 2022 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customise favicon and title for dedicated repositories [2]
5 participants