Skip to content

Good riddance#3367

Merged
mathjazz merged 11 commits into
mozilla:mainfrom
mathjazz:admin-tag-management
Sep 26, 2024
Merged

Good riddance#3367
mathjazz merged 11 commits into
mozilla:mainfrom
mathjazz:admin-tag-management

Conversation

@mathjazz
Copy link
Copy Markdown
Collaborator

@mathjazz mathjazz commented Sep 22, 2024

Fix #3108.

  1. First and foremost, this patch replaces the unusual widget for selecting tag resources in Pontoon Admin with the multiple item selector that is more commonly used in Pontoon (see Add new Tag resource management UI).

  2. Secondly, the last bits of the old Tags Django code are now finally removed. It should now be much simpler to fix issues like Add ability to define a default tag per project #2122 and Provide a way to analyze tags for errors #2131.

  3. Finally, the odd tag-admin frontend, which was only powering the old widget for selecting tag resources in Pontoon Admin, is now removed (see Remove Tag Admin Frontend code and Remove Tag Admin Frontend environment).

In total, the same (actually, I think it's much better) functionality uses some 95% smaller footprint:
Screenshot 2024-09-22 at 16 23 43

The patch was tested locally in Docker.

It should also be tested on stage before we can proceed:
https://mozilla-pontoon-staging.herokuapp.com/admin/projects/firefox/

Also included are a few smaller Pontoon Admin improvements:

A few additional changes would be nice to make in Pontoon Admin. At least one of them would clash with #3353, so I've filed #3368 for that.

@mathjazz mathjazz requested a review from eemeli September 23, 2024 06:47
@flodolo
Copy link
Copy Markdown
Collaborator

flodolo commented Sep 26, 2024

This patch should be tested on stage before we can proceed: mozilla-pontoon-staging.herokuapp.com/admin/projects/firefox

I assume this isn't on stage anymore at this point?

@mathjazz
Copy link
Copy Markdown
Collaborator Author

mathjazz commented Sep 26, 2024

I assume this isn't on stage anymore at this point?

It will be again in a few mintues, just finishing deploying #3378, #3353 and #3367.

Update: deployed.

@flodolo
Copy link
Copy Markdown
Collaborator

flodolo commented Sep 26, 2024

Unify dropbox style with the rest of the UI

I don't remember if we have docs with screenshots of the admin UI. In case, they will need to be redone (I like the more consistent look).

  • Available-Chosen: should we stick to Linked instead of Chosen?
  • Unlike in the locales section, there is no visual representation (< or > on hover) that clicking will move an item in the other column. Is that by design?

Something is broken though: I moved one file from mobile into one tag, and now the list of resources doesn't load anymore.
https://mozilla-pontoon-staging.herokuapp.com/ace/firefox/

EDIT: I think this is the log

  File "/app/pontoon/base/utils.py", line 127, in wrap
    return f(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/pontoon/localizations/views.py", line 101, in ajax_resources
    resource_priority_map = project.resource_priority_map()
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/pontoon/base/models/project.py", line 414, in resource_priority_map
    and resource_priority[path] >= item["priority"]
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '>=' not supported between instances of 'NoneType' and 'int'

@mathjazz
Copy link
Copy Markdown
Collaborator Author

I don't remember if we have docs with screenshots of the admin UI. In case, they will need to be redone (I like the more consistent look).

I only found this: https://mozilla-l10n.github.io/documentation/tools/pontoon/adding_new_project.html

The screenshot only contains the tag management part of the Admin UI, so we can update it (and the corresponding text) as soon as this PR is merged.

Available-Chosen: should we stick to Linked instead of Chosen?

This is currently hardcoded in the widget. Due to the clash with #3353, we could implement it as part of #3368 (or a separate issue).

Unlike in the locales section, there is no visual representation (< or > on hover) that clicking will move an item in the other column. Is that by design?

The locale widget is specific, because an arrow is a must in the middle column. In other places where the widget is used, we don't use an arrow in order to save space, see user settings or Messaging center.

We could still add an arrow, but for the reason mentioned above, that would need to be in #3368 (or a separate issue).

Something is broken though: I moved one file from mobile into one tag, and now the list of resources doesn't load anymore.

Nice catch! Looking into it.

@mathjazz
Copy link
Copy Markdown
Collaborator Author

Nice catch! Looking into it.

Update: Clicking on a resource unsets the tag priority, and the dashboard breaks in case tags don't have a priority. Interestingly though I can't reproduce that locally yet.

We should also make tag priority a required field (because even on main, if you remove priority from tag by mistake, the dashboard will break).

@mathjazz
Copy link
Copy Markdown
Collaborator Author

Update: Clicking on a resource unsets the tag priority, and the dashboard breaks in case tags don't have a priority. Interestingly though I can't reproduce that locally yet.

Turns out this is a bug in #3353, which is also deployed to stage. 🤣

@mathjazz
Copy link
Copy Markdown
Collaborator Author

@flodolo Updated the stage with fixes made in #3353.

@flodolo
Copy link
Copy Markdown
Collaborator

flodolo commented Sep 26, 2024

The admin area looks good on stage. Is there anything else that needs testing?

@mathjazz
Copy link
Copy Markdown
Collaborator Author

The admin area looks good on stage. Is there anything else that needs testing?

Thanks. Mostly Admin, but also the Tag dashboards.

Copy link
Copy Markdown
Collaborator

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

Both admin area and tag dashboard look good to me.

Comment thread pontoon/administration/forms.py Outdated
Comment thread pontoon/administration/static/css/admin_project.css
Comment thread pontoon/settings/base.py
Comment thread pontoon/base/models/resource.py
Comment thread pontoon/settings/base.py
Co-authored-by: Francesco Lodolo <flod@lodolo.net>
@mathjazz mathjazz removed the request for review from eemeli September 26, 2024 15:56
@mathjazz mathjazz merged commit bad34f1 into mozilla:main Sep 26, 2024
@mathjazz mathjazz deleted the admin-tag-management branch September 26, 2024 16:01
@bcolsson
Copy link
Copy Markdown
Collaborator

https://www.youtube.com/watch?v=CnQ8N1KacJc

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.

Refactor and redesign Tags Admin

3 participants