Skip to content
This repository has been archived by the owner on Jan 10, 2024. It is now read-only.

SJRK-297: removing edit page links since story gathering is complete #47

Merged
merged 8 commits into from Nov 18, 2019

Conversation

BlueSlug
Copy link
Member

This change is meant to be merged into the stories-floe-dev branch and thence immediately into stories-sojustrepairit-production.

It should also be accompanied by updates to the configuration file for the SJRK Storytelling Tool:

"globalConfig": {
    "theme": "sojustrepairit",
    "authoringEnabled": false
}

@@ -7,5 +7,4 @@
<li><a href="storyBrowse.html">{{localizedMessages.link_browse_text}}</a></li>
</ul>
</nav>
<a class="sjrk-st-edit-link" href="{{localizedMessages.editor_url}}">{{localizedMessages.link_edit_text}}</a>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the showing and hiding of this link be controlled by authoringEnabled flag defined in the config file? Or sojustrepairit site will not allow authoring from now on?

Copy link
Member Author

@BlueSlug BlueSlug Nov 15, 2019

Choose a reason for hiding this comment

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

@cindyli this is an interesting question. In the case of the learningReflections theme, we still want to show the button and link it to the staging site, but in the case of cities and sojustrepairit, we do indeed want to hide it when authoringEnabled is false.

I'm hesitant to tie it directly to the config value at this point, at least not until we've dealt with SJRK-52, as the duplicate theme-specific templates may collapse into partials that are reused and shared between themes.

Copy link
Member

Choose a reason for hiding this comment

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

in the case of cities and sojustrepairit, we do indeed want to hide it when authoringEnabled is false.

I think What you said reinforces the point that the showing/hiding of this link should be controlled by the authoringEnabled config. Due to the different handlings for different themes, when dealing with SJRK-52, the condition on authoringEnabled suggests this part pertains to theme-specific templates rather than shared templates.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cindyli I attempted to make the change you suggested, but discovered a bug in how the authoringEnabled flag is relayed to templates. It seems only the templates on the Edit page receive that value as expected. I've captured the issue: https://issues.fluidproject.org/browse/SJRK-315

With this in mind, can this change be deferred to that Jira?

Copy link
Member

Choose a reason for hiding this comment

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

@BlueSlug, how urgent do we need to lock down the story editing for the SJRK Stories site? If it's urgent, deferring the fix to SJRK-315 is OK, otherwise, I hope we could address the issue here if possible to avoid fixing one issue leading to the open of one or several new JIRAs.

I looked into the distributeOptions issue on the browsing page. Seems it was caused by a timing issue as the initialization of authoringEnabled is dependent on the completion of /clientConfig request. To fix it, the instantiation of the page component should wait until /clientConfig request finishes fetching config values from the server.

Copy link
Member Author

@BlueSlug BlueSlug Nov 18, 2019

Choose a reason for hiding this comment

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

@cindyli I'm having trouble understanding the nature of the race condition. After poring over the code Friday and today, I'm still unable to see it and the promise structure looks okay to me. Perhaps we could pair up on this and go through it together?

Copy link
Member

Choose a reason for hiding this comment

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

@BlueSlug, I issued this pull request against your branch. It should fix the race condition.

@BlueSlug
Copy link
Member Author

Ready to proceed with merging and locking down authoring for the SJRK Storytelling Tool

@BlueSlug
Copy link
Member Author

Ready for more review

@cindyli cindyli merged commit 66ab083 into fluid-project:stories-floe-dev Nov 18, 2019
@cindyli
Copy link
Member

cindyli commented Nov 18, 2019

Merged at 81cdcdb

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants